Opened 4 months ago

Last modified 4 months ago

#15294 new defect

AGS: Frequent use of bitwise OR/ANDs instead of logical operators [upstream "bug"]

Reported by: RainRat Owned by:
Priority: normal Component: Engine: AGS
Version: Keywords:
Cc: Game:

Description (last modified by RainRat)

In scummvm\engines\ags\engine\ac\global_slider.cpp the SetSliderValue and GetSliderValue functions, the validation checks for the GUI number (guin) are using the bitwise OR operator (|) instead of the logical OR operator (||).

Affected Code:

void SetSliderValue(int guin, int objn, int valn) {
    if ((guin < 0) | (guin >= _GP(game).numgui)) quit("!SetSliderValue: invalid GUI number");
[...]
int GetSliderValue(int guin, int objn) {
    if ((guin < 0) | (guin >= _GP(game).numgui)) quit("!GetSliderValue: invalid GUI number");

Corrected Code:

void SetSliderValue(int guin, int objn, int valn) {
    if ((guin < 0) || (guin >= _GP(game).numgui)) quit("!SetSliderValue: invalid GUI number");
[...]
int GetSliderValue(int guin, int objn) {
    if ((guin < 0) || (guin >= _GP(game).numgui)) quit("!GetSliderValue: invalid GUI number");

Change History (4)

comment:1 by RainRat, 4 months ago

Description: modified (diff)

comment:2 by tag2015, 4 months ago

Thanks for the report!
This is an OR between booleans so although it's not good practice it shouldn't change anything (besides a minimal speed improvement)

There are quite a lot of similar occurrences in the AGS codebase (also with &) so we have to decide if we should fix them all or leave them as is...

I'll bump the priority to keep track, then we'll see

comment:3 by tag2015, 4 months ago

Priority: lownormal
Summary: AGS: Incorrect Use of Bitwise OR Instead of Logical OR in SetSliderValueAGS: Frequent use of bitwise OR/ANDs instead of logical operators [upstream "bug"]

comment:4 by RainRat, 4 months ago

If I find any more, I'll add them here, so they're in one place:

scummvm\engines\ags\engine\gui\new_control.cpp
Current:

int NewControl::mouseisinarea(int mx, int my) {
[...]
	if ((mx > x) &(mx < x + wid) &(my > y) &(my < y + hit))

Expected:

int NewControl::mouseisinarea(int mx, int my) {
[...]
	if ((mx > x) &&(mx < x + wid) &&(my > y) &&(my < y + hit))
Last edited 4 months ago by RainRat (previous) (diff)
Note: See TracTickets for help on using tickets.