fix(weaponset): Add sanity check for weapon slot index in WeaponSet::setWeaponLock()

[Caball009]

The following GameMessage types read a weapon slot index from the game messages:

GameMessage::MSG_DO_WEAPON
GameMessage::MSG_DO_WEAPON_AT_OBJECT
GameMessage::MSG_SWITCH_WEAPONS
GameMessage::MSG_DO_WEAPON_AT_LOCATION

There's currently no sanity check on the value, so it may be modified to go to cause the game access out-of-bounds data and cause a crash. This PR adds a sanity check before the weapon slot value is used.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0041b1a8-7e31-4baa-bccf-7e29946f0c49

📥 Commits

Reviewing files that changed from the base of the PR and between b234b3ed48d61abfe47dcee407c5e6a590b77f02 and 4883006.

📒 Files selected for processing: 2

• Generals/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp
• GeneralsMD/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

Walkthrough

Both game variants now check that a weapon slot is within the weapon array before setWeaponLock accesses it. The method returns false for an invalid slot.

Changes

Weapon lock slot validation

Layer / File(s)	Summary
Reject invalid weapon slots Generals/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp, GeneralsMD/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp	setWeaponLock returns false when the weapon slot is outside the weapon array, before accessing the array. Other lock handling is unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: bobtista

Merge Risk: ⚪ Minimal · up to 48830

Both game variants now reject invalid weapon slots before accessing the weapon array, preventing the reported crash path while preserving valid-slot behavior. No material merge risk remains.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly identifies the main change: adding a weapon-slot index sanity check in WeaponSet::setWeaponLock.
Description check		The description explains the unchecked weapon-slot values, the affected GameMessage types, the out-of-bounds risk, and the added sanity check.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Adds bounds checks to WeaponSet::setWeaponLock in both games.

• Weapon lock requests check the slot before accessing the weapon array.

_{Reviews (2) · Last reviewed commit: "fix(weapon): Add sanity check for weapon..." · Reviewed by Greptile}

[Skyaero42]

Is a DEBUG_CRASH maybe warranted here?

[Caball009]

I don't mind adding it, but I don't see the value myself.

[Mauller]

It can help warn of invalid data?

[Caball009]

It can help warn of invalid data?

What is the value of the warning? Is there an action we could or should take if we know that this code was executed?

[Mauller]

i was just considering if there was the possibility of invalid ini data getting this far, but it likely doesn't.

You could also simplify this to weaponSlot >= WEAPONSLOT_COUNT

[Caball009]

i was just considering if there was the possibility of invalid ini data getting this far, but it likely doesn't.

Not that I'm aware of.

You could also simplify this to weaponSlot >= WEAPONSLOT_COUNT

This doesn't catch negative numbers (WeaponSlotType's underlying type is signed). It also requires you to look up that ARRAY_SIZE(m_weapons) == WEAPONSLOT_COUNT.

[Mauller]

most of the code already uses WEAPONSLOT_COUNT in this context, but it is also fair on the unsigned side of it.

[xezon]

Makes sense.