Skip to content

Comments

円錐型当たり判定ライブラリの実装#2211

Open
komaramune wants to merge 1 commit intomasterfrom
dev/komaramune_bounding_cone
Open

円錐型当たり判定ライブラリの実装#2211
komaramune wants to merge 1 commit intomasterfrom
dev/komaramune_bounding_cone

Conversation

@komaramune
Copy link
Contributor

No description provided.

@komaramune komaramune requested a review from ChenCMD as a code owner December 30, 2025 09:52
Copilot AI review requested due to automatic review settings December 30, 2025 09:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a cone-shaped bounding volume detection library for Minecraft entities. The implementation allows users to detect entities within a conical region defined by an apex point, axis direction, and apex angle.

  • Adds a new public API (lib:bounding_cone/) for cone-shaped collision detection
  • Implements geometric calculations using trigonometry and entity rotation manipulation
  • Uses a macro-based detection system similar to the existing bounding_fan library

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
TheSkyBlessing/data/lib/functions/bounding_cone/.mcfunction Main API entry point with argument validation and documentation for the cone detection system
TheSkyBlessing/data/lib/functions/bounding_cone/core/calc.mcfunction Core calculation logic that computes sin(angle/4) and prepares data for macro-based detection
TheSkyBlessing/data/lib/functions/bounding_cone/core/detect.m.mcfunction Macro function that performs geometric detection using entity positioning and rotation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

#
# @within function lib:bounding_cone/

# sin(頂角/4) 取得
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "sin(頂角/4) 取得" (getting sin(apex angle/4)), but the typical approach for cone detection would use sin(apex angle/2) to represent the half-angle from the axis to the edge. While this might be intentional based on the specific geometric algorithm used (as seen in the similar bounding_fan implementation), the documentation should explain why angle/4 is used instead of the more conventional angle/2 to help future maintainers understand the mathematical reasoning.

Suggested change
# sin(頂角/4) 取得
# sin(頂角/4) 取得
# 注意:
# 一般的な円錐判定では「軸から母線までの半角 θ/2 」を用いて sin(θ/2) を計算することが多いが、
# この bounding_cone 実装では bounding_fan と同じ幾何アルゴリズムに基づき、
# マクロ側の計算都合上「頂角 θ に対して sin(θ/4)」を事前に求める設計としている。
# そのため、ここを安易に θ/2 に変更すると、detect.m 内の判定ロジックとの整合性が崩れる点に注意。

Copilot uses AI. Check for mistakes.
#
# @within function lib:bounding_cone/

# sin(頂角/4) 取得
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The storage path uses inconsistent terminology between English and Japanese. Line 5 references storage lib: Argument.BoundingCone.Angle but the comment uses Japanese "頂角". For maintainability, consider adding a comment explaining that "Angle" refers to the cone's apex angle (頂角) to make the code more understandable for both Japanese and English readers.

Suggested change
# sin(頂角/4) 取得
# sin(頂角/4) 取得
# Note: Argument.BoundingCone.Angle represents the cone's apex angle (頂角).

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@EllaCoat EllaCoat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こまさんに聞きながらコマンドチェックを行った所、コマンド自体にミスは見られなかったため、私はApproveをします。

@EllaCoat
Copy link
Contributor

とりあえず、はいいろ氏辺りに今度見てもらって大丈夫そうなら使おうかな。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants