Skip to content

Fix multiple issues in sub_6FC6A8C0#219

Open
AudriusButkevicius wants to merge 1 commit into
ThePhrozenKeep:masterfrom
AudriusButkevicius:fix/194-wrong-index-calc
Open

Fix multiple issues in sub_6FC6A8C0#219
AudriusButkevicius wants to merge 1 commit into
ThePhrozenKeep:masterfrom
AudriusButkevicius:fix/194-wrong-index-calc

Conversation

@AudriusButkevicius

Copy link
Copy Markdown
Contributor

Summary

  1. Make coordinate array static const for immutability and performance
  2. Fix incorrect index calculation 2 * (nParam1 + nParam2)nParam1 + nParam2, preventing OOB reads
  3. Hoist monCreate initialization outside the loop (only X/Y change per iteration)
  4. Fix bounds check a6 > 2a6 > 1 to prevent OOB reads when a6 == 2 (vanilla bug)

Fixes #194

Note: This PR was produced using Claude Code based on the upstream issue description. The change has not been compiled or tested, as the project requires a Windows build environment which was not available. Please review carefully before merging.

1. Make coordinate array static const
2. Fix incorrect index calculation (2 * (nParam1 + nParam2) -> nParam1 + nParam2)
3. Hoist monCreate initialization outside loop
4. Fix OOB bounds check (a6 > 2 -> a6 > 1)

Fixes ThePhrozenKeep#194
@@ -1538,7 +1538,7 @@ int32_t __fastcall sub_6FC6A810(D2GameStrc* pGame, D2ActiveRoomStrc* pRoom, int3
//D2Game.0x6FC6A8C0
int32_t __fastcall sub_6FC6A8C0(D2GameStrc* pGame, D2UnitStrc* pUnit, int32_t nMonsterId, int32_t nAnimMode, int32_t nCount, int32_t a6, int16_t nFlags)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

According to the issue description from Nec, we can still rename a6 to bNonWaterSpawn


int32_t v9 = a6;
if (a6 < 0 || a6 > 2)
if (a6 < 0 || a6 > 1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Since the Vanilla code actually really looks like this, I don't know if @Lectem would like to leave this inside a #ifdef NO_BUG_FIX, but it probably doesn't make a lot of sense 😄

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.

Multiple issues with sub_6FC6A8C0

3 participants