Fixed infantry 'Boxing' Mechanic #45
Conversation
- Fixed use of missing 'BodyFacing' variable in the drawing code - Fixed use of missing 'Establish_Contact' method, used 'Transmit_Message(RADIO_HELLO...' instead d2f1d8
- Target was only being set after call to 'Do_Action(DO_ON_GUARD...' which caused facing to be calculated incorrectly, as the target was invalid at the time of the call. 0731dc
- Randomized reading of infantry punch and kick damage arrays provided a max value that one too high, and was our of the range of the array.
- This issue occurred when an infantry was killed back by the explosive death of a unit it just won a boxing match against
- Similar to when infantry are firing, they shouldn't be moving when they are boxing
For a number of reasons, infantry can become stuck in one of the boxing states, while they have been told to stop boxing. This check catches those cases.
| @@ -2786,15 +2810,15 @@ RadioMessageType InfantryClass::Receive_Message(RadioClass * from, RadioMessageT | |||
| ** Just received a kick! Take some damage. | |||
| */ | |||
| case RADIO_KICK: | |||
| damage = Infantry_Kick_Damage[Random_Pick(0, (int)(sizeof(Infantry_Kick_Damage) / sizeof(Infantry_Kick_Damage[0])))]; | |||
| damage = Infantry_Kick_Damage[Random_Pick(0, (int)(sizeof(Infantry_Kick_Damage) / sizeof(Infantry_Kick_Damage[0])) - 1)]; | |||
There was a problem hiding this comment.
A classic off-by-one error. I wonder how many of those invalid reads are in the code.
Maybe create a macro/inline template to properly access such arrays?
#define MAX_ARRAY_INDEX(array) (int)(sizeof(array) / sizeof(array[0])) - 1I out of C++ for several years but I am sure an inline template function could make this even more secure.
There was a problem hiding this comment.
Yea, that's what I would have done ideally if this was a more active codebase. I was trying to fix these issues while touching as little as possible so it would be straight-forward for others to integrate without conflicts.
There was a problem hiding this comment.
A classic off-by-one error. I wonder how many of those invalid reads are in the code.
Maybe create a macro/inline template to properly access such arrays?#define MAX_ARRAY_INDEX(array) (int)(sizeof(array) / sizeof(array[0])) - 1I out of C++ for several years but I am sure an inline template function could make this even more secure.
You probably want to wrap that inside another pair of parentheses in case you ever want to use an operator with a higher priority after using this macro. It is a little bit nitpicky and I don't see a real use of this happening in this project, but I prefer to be extra cautious when using macros.
Instead of:
#define MAX_ARRAY_INDEX(array) (int)(sizeof(array) / sizeof(array[0])) - 1
Use:
#define MAX_ARRAY_INDEX(array) ((int)(sizeof(array) / sizeof(array[0])) - 1)
A situation like this could return undesired side-effects (imagine an array size of 5, thus max index 4):
int a = MAX_ARRAY_INDEX(array) * 2; The intended value of 'a' is 8.
Would be understood as:
int a = (int)(sizeof(array) / sizeof(array[0])) - 1 * 2; The actual value of 'a' is 3.
There was a problem hiding this comment.
Correct, my code was made up on the fly and I mentioned there is probably a more typesafe way in C++ to do that.
Fixed compile errors and bugs in cut infantry boxing/hand-to-hand/fisticuffs mechanic. The mechanic will now function as intended when compiling with "BOXING" defined.