 |
Ok I completed the audit. Full results attached. First, the rules: 1. Script events and entity targets start counting objectives at 1, to align to DR 2. Game code in c++ starts counting at zero, to align to c++ 3. Code that handles script events should therefore subtract 1 from the number passed by the scripter or the target, before calling an internal function that starts counting at 0 4. c++ code uses the internal function, not the script event handler 5. bounds must be checked correctly Table of results attached, covering the script events, their callers, their calls to internal handlers, the bounds checks in the internal handlers, and the callers of the internal handlers. Also checked that objective and component numbers are handled ok in the rest of the module. The good news is that there's no calling code anywhere else in the codebase that needs changing. Likewise no published maps. All entity targets are using the indexes ok. Changes can be restricted to the script events in player.cpp and the handlers in MissionData.cpp. 5 script events pass a bad parameter, and 7 internal handlers have bad bounds checks. In addition to that, one script event (Event_SetObjectiveComp) uses a non-standard setup. It looks like an earlier design before the current convention was established. It's not broken but it breaks rules 2, 3, and 4 by using an intermediate handler that does the index subtraction and the bounds check. And that intermediate is called from one other place in the c++ code (Target_SetObjectiveComponentState), which is the only place in the c++ code that passes a 1-based index and isn't a bug. Comparable entities like Ctarget_SetObjectiveState follow the usual 0-based rules. Given there are no callers of the intermediate except for that target and the script event, I suggest we eliminate it and move the bounds check to the internal handler, then everything will work the same way. All the rest of the internal handlers have bounds checks, again this one is an odd one out. |
|
|
Fixed at rev 6075. Updated file attached. |
- Anonymous
