Attnam / ivan

Iter Vehemens ad Necem - a continuation of the graphical roguelike by members of http://attnam.com
GNU General Public License v2.0
302 stars 42 forks source link

Character states overflows after 32 states #353

Open ryfactor opened 6 years ago

ryfactor commented 6 years ago

https://github.com/Attnam/ivan/blob/3e05963d2fd2f126b8dddbcacba2d755c2ecda99/Main/Include/ivandef.h#L158

@emlai we're looking for a solution to a technical problem with character state representation in IVAN. Basically, we can't add more than 32 states. Add another one and it overflows to POLYMORPHED. This is due to the use of 32-bit integer numbers being used as fast flags to represent each flag. We could theoretically use a long long int but this would only give us an additional 32 states.

Do you have any suggestions as to how we could improve state representation in a more guru way? We have a thread in the usual place: https://attnam.com/topics/Overflow

AquariusPower commented 6 years ago

ex.: to help on debug std::bitset<64>(character->GetFlags()) I think any value can be used instead of 64

https://softwareengineering.stackexchange.com/a/284166 as the questioner said too, I also cringe when seeing bitmask shiftings :)

so, I suggest trying to use bitset, make a simple 1000000 loop performance test (disable CPU throttle on your OS or it will be imprecise probably) to be sure it is perf wise.

The biggest advantage is preventing mistakes on bit shifting calculations, and to make it easier for new developers to provide patches :)

Obs.: I haven't tested (as I usually do b4 posting anything) using std::bitset to "actually" set the bits but I am quite sure that will work :D

EDIT: http://www.cplusplus.com/reference/bitset/bitset/ each method there is a link pointing to more explanations and examples, we will use mostly: set() test() flip()

I have tried this: g++ -c testStdBitset.cpp g++ -o testStdBitset testStdBitset.o

#include <iostream>
#include <bitset>

int main()
{
  std::bitset<128> bsStates;
  bsStates.reset(); std::cout << bsStates << std::endl;
  bsStates.set(4); std::cout << bsStates << std::endl;
  bsStates.set(7); std::cout << bsStates << std::endl;
  return 0;
}

outputs

00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000
00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010010000
ryfactor commented 6 years ago

@AquariusPower that's great. It looks like it supports all the operations we need. How much work is there in replacing C-style bit-bashing with std::bitset? We can limit the scope of the changes to only the character states subsystem for starters.

AquariusPower commented 6 years ago

by what I read here: http://www.cplusplus.com/reference/bitset/bitset/operators/ , at the examples, it seems we may be able to keep the code unmodified concerning "C-style bit-bashing", only requiring to change the variable and the methods' return types.