Add cube skill support and debug visuals#649
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe changes introduce cube skill support across the data and runtime layers. A new Changes
Sequence Diagram(s)sequenceDiagram
participant MapParser as MapEntity<br/>Parser
participant FM as FieldManager
participant FS as FieldSkill
participant SU as SkillUtils
MapParser->>FM: Init(entities: MapEntityMetadata)
FM->>FM: Loop CubeSkills from metadata
FM->>FM: Lookup SkillMetadata by SkillId
FM->>FM: AddCubeSkill(metadata, interval,<br/>position, rotation)
FM->>FS: Create FieldSkill instance
FM->>FM: Store in cubeSkills dict
Note over FM: Per Frame Update
FM->>FS: Update(FieldTick)
FS->>SU: GetPrism(position, skillAngle,<br/>applyTarget)
SU->>FS: Return prism with<br/>region-buff centering
FS->>FS: Evaluate targets within prism
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
Maple2.Server.DebugGame/Graphics/DebugFieldRenderer.cs (1)
300-307:zOffsetis always 0 — consider removing the parameter or documenting its purpose.Both call sites pass
zOffset = 0. If this is a placeholder for future use, a brief comment in the method or at the call site would help. Otherwise, the local variable declarations add noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Maple2.Server.DebugGame/Graphics/DebugFieldRenderer.cs` around lines 300 - 307, The local zOffset variable is always zero at both call sites (ShowFieldSkillHitboxes and ShowCubeSkillHitboxes) and is passed unchanged into RenderFieldSkillHitboxes; either remove the zOffset parameter from RenderFieldSkillHitboxes and update all callers (including the calls using Field.DebugFieldSkills and Field.DebugCubeSkills) to call the method without zOffset, or if the parameter is intended for future use, add a concise comment on the RenderFieldSkillHitboxes method explaining its planned purpose and keep the calls simple (remove the local zOffset declarations at the call sites).Maple2.File.Ingest/Mapper/MapEntityMapper.cs (1)
183-189: Hardcoded interval500is a magic number.Consider extracting this to a named constant (e.g.,
const int CubeSkillDefaultInterval = 500;) for clarity and easier future adjustment. This is consistent with how it's also used in the existingMs2RegionSkillpath whereskill.Intervalcomes from data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Maple2.File.Ingest/Mapper/MapEntityMapper.cs` around lines 183 - 189, The Ms2CubeSkill creation in MapEntityMapper.cs uses the magic number 500 for the interval; define a named constant (e.g., const int CubeSkillDefaultInterval = 500) near the MapEntityMapper class and replace the literal 500 in the Ms2CubeSkill constructor call inside the IMS2CubeProp handling block (the physXProp pattern that yields new MapEntity(...) with Block = new Ms2CubeSkill(...)) to use CubeSkillDefaultInterval so the intent is clear and adjustable.Maple2.Server.DebugGame/Graphics/Ui/Windows/VisualizationControlsWindow.cs (1)
58-60: Consider grouping "Field Skill" and "Cube Skill" hitboxes under "Show Skill Hitboxes" for UX consistency.Every other parent-child pair in this window (Portals → Portal Information/Connections, Triggers → Trigger Information, Actors → Players/NPCs/Mobs) follows the pattern:
ImGui.Checkbox("Show Parent", ref renderer.ShowParent); if (renderer.ShowParent) { ImGui.Indent(); ImGui.Checkbox("Show Child", ref renderer.ShowChild); ImGui.Unindent(); }If field-skill and cube-skill hitboxes are sub-visualizations of skill hitboxes, apply the same grouping:
♻️ Proposed grouping
ImGui.Checkbox("Show Skill Hitboxes", ref renderer.ShowSkillHitboxes); -ImGui.Checkbox("Show Field Skill Hitboxes", ref renderer.ShowFieldSkillHitboxes); -ImGui.Checkbox("Show Cube Skill Hitboxes", ref renderer.ShowCubeSkillHitboxes); +if (renderer.ShowSkillHitboxes) { + ImGui.Indent(); + ImGui.Checkbox("Show Field Skill Hitboxes", ref renderer.ShowFieldSkillHitboxes); + ImGui.Checkbox("Show Cube Skill Hitboxes", ref renderer.ShowCubeSkillHitboxes); + ImGui.Unindent(); +}If they are intentionally independent visualizations, no change is needed — but a visual
ImGui.Separator()between the three would help convey that.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Maple2.Server.DebugGame/Graphics/Ui/Windows/VisualizationControlsWindow.cs` around lines 58 - 60, The three flat checkboxes should follow the existing parent-child pattern: treat ShowFieldSkillHitboxes and ShowCubeSkillHitboxes as children of ShowSkillHitboxes. Replace the three independent ImGui.Checkbox calls with a parent ImGui.Checkbox for renderer.ShowSkillHitboxes and wrap the field/cube checkbox calls inside if (renderer.ShowSkillHitboxes) { ImGui.Indent(); /* child checkboxes: renderer.ShowFieldSkillHitboxes, renderer.ShowCubeSkillHitboxes */ ImGui.Unindent(); } so the two become visually nested under the skill hitboxes toggle; if you intentionally want them independent instead, add an ImGui.Separator() between the parent and the others to improve visual separation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Maple2.File.Ingest/Mapper/MapEntityMapper.cs`:
- Around line 183-189: The Ms2CubeSkill creation in MapEntityMapper.cs uses the
magic number 500 for the interval; define a named constant (e.g., const int
CubeSkillDefaultInterval = 500) near the MapEntityMapper class and replace the
literal 500 in the Ms2CubeSkill constructor call inside the IMS2CubeProp
handling block (the physXProp pattern that yields new MapEntity(...) with Block
= new Ms2CubeSkill(...)) to use CubeSkillDefaultInterval so the intent is clear
and adjustable.
In `@Maple2.Server.DebugGame/Graphics/DebugFieldRenderer.cs`:
- Around line 300-307: The local zOffset variable is always zero at both call
sites (ShowFieldSkillHitboxes and ShowCubeSkillHitboxes) and is passed unchanged
into RenderFieldSkillHitboxes; either remove the zOffset parameter from
RenderFieldSkillHitboxes and update all callers (including the calls using
Field.DebugFieldSkills and Field.DebugCubeSkills) to call the method without
zOffset, or if the parameter is intended for future use, add a concise comment
on the RenderFieldSkillHitboxes method explaining its planned purpose and keep
the calls simple (remove the local zOffset declarations at the call sites).
In `@Maple2.Server.DebugGame/Graphics/Ui/Windows/VisualizationControlsWindow.cs`:
- Around line 58-60: The three flat checkboxes should follow the existing
parent-child pattern: treat ShowFieldSkillHitboxes and ShowCubeSkillHitboxes as
children of ShowSkillHitboxes. Replace the three independent ImGui.Checkbox
calls with a parent ImGui.Checkbox for renderer.ShowSkillHitboxes and wrap the
field/cube checkbox calls inside if (renderer.ShowSkillHitboxes) {
ImGui.Indent(); /* child checkboxes: renderer.ShowFieldSkillHitboxes,
renderer.ShowCubeSkillHitboxes */ ImGui.Unindent(); } so the two become visually
nested under the skill hitboxes toggle; if you intentionally want them
independent instead, add an ImGui.Separator() between the parent and the others
to improve visual separation.
| if (physXProp is IMS2CubeProp { skillID: > 0, skillLevel: > 0 } cubeProp) { | ||
| yield return new MapEntity(xblock, new Guid(entity.EntityId), entity.EntityName) { | ||
| Block = new Ms2CubeSkill(cubeProp.skillID, (short) cubeProp.skillLevel, | ||
| 500, cubeProp.Position, cubeProp.Rotation), | ||
| }; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Considering this is far up the switch case, I am concerned this is catching more blocks than expected. Has it been checked if, for example, an interactable mesh isn't being caught up in this grouping instead?
2 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements