DCurrent / openbor

OpenBOR is the ultimate 2D side scrolling engine for beat em' ups, shooters, and more!
http://www.chronocrash.com
BSD 3-Clause "New" or "Revised" License
901 stars 121 forks source link

Bitwise Not Nonfunctional #257

Closed DCurrent closed 2 years ago

DCurrent commented 2 years ago

Bitwise not (~) is unusable and throws erroneous error.

Description

Any attempted use of ~ results in error at load.

Debugging

Reproduce

  1. Add ~ operator to any module script. Example: flag_state &= ~openborconstant("IN_SCREEN_OPTIONS_MENU")
  2. Load module.
  3. Module will fail to load, returning error downstream of operator:

"Script compile error in 'data/scripts/dc_damage/damage.c': openborconstant line 50, column 23"

Expected behavior

~ operator appears in source code but does not function at all. I've looked through parser and attempted to debug, but thus far have been unable to find root cause. Other bit operators such as ^ work as expected.

Plombo commented 2 years ago

I don't know why it's failing to compile the script, but from a look at Parser.c, it's very obvious that the implementation of the ~ operator is completely wrong anyway. As in, the person who implemented the operator (take a guess) clearly didn't even know what it's supposed to do.

DCurrent commented 2 years ago

I wondered about that. Almost looks like it was set up as a simple sign inversion, but I just chalked it up to my missing something. If you have any ideas what I could do about it I'm all ears. I've been working to compact a lot of binary state flags into one value, and it works really nice in source code, but without a ~ operator script access gets over-complicated. If you want to ensure a bit is off, you have to get the value, check to see if bit is true, then toggle it with ^=.

Plombo commented 2 years ago

Yeah, I'd also think I was missing something if I weren't already familiar with the code. It's not even implementing ~ as a "simple" negation, really, but as a prefix/postfix operator that modifies the variable like ++ and -- do. Seeing it grouped with those two operators in the source code was how I first noticed that something was seriously wrong.

Fortunately, I don't think it will be too hard to throw that code out and implement it properly.

DCurrent commented 2 years ago

It's not even implementing ~ as a "simple" negation, really, but as a prefix/postfix operator that modifies the variable like ++ and -- do.

...and that guy claims to have a PhD in Computer Science. Oh well, thanks for checking this out @Plombo!

Plombo commented 2 years ago

@DCurrent, can you check out the "bitwise-not" branch I just pushed and see if it works? (I made the changes in 5 minutes and haven't exactly tested it myself, so there's maybe a 50/50 chance.)

DCurrent commented 2 years ago

So sorry @Plombo, I just noticed this. I'm away from my desk and have a meeting first thing in the morning. I'll try to check it out by tomorrow afternoon.

Plombo commented 2 years ago

Sure, no hurry. After all, I'm the one who took 3 weeks to get around to a 5-minute change. 🙃️

DCurrent commented 2 years ago

Works beautifully, thank you @Plombo! I used a simple test with the new cheat system. Defaulted touch of death cheat on, then turned it off with bitwise not. Confirmed through menu and also via log output:

void global_config = openborvariant("global_config");
int cheats = get_global_config_property(global_config, "cheats");

log("\n\n global_config: " + global_config);
log("\n\t cheats: " + cheats);

cheats &= ~openborconstant("CHEAT_OPTIONS_TOD_ACTIVE");

log("\n\t cheats: " + cheats);

set_global_config_property(global_config, "cheats", cheats);

Log output:

global_config: #6816532
     cheats: 30122
     cheats: 21930

Unless you have reason not to, I'll roll this into the global beta and flag the commit as a fix for this issue. Thanks again!

Plombo commented 2 years ago

Glad to hear it works!