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

confdef.h and define.dat are not in sync #379

Closed AquariusPower closed 6 years ago

AquariusPower commented 6 years ago

as you can see on the picture (wizard mode), selection_095 there are 2 missplaced chairs created by the eddys, and a couch created by them too.

define.dat

#define COUCH 5
#define CHAIR 6
#define DOUBLE_BED 9

confdef.h

#define COUCH 5
#define DOUBLE_BED 6

eddy code:

void eddy::GetAICommand() ...
    decoration* Couch = decoration::Spawn(RAND_N(5) ? COUCH : DOUBLE_BED);

as confdef.h and define.dat are not in sync, eddy should have spawned config value 9 from define.dat as the identifier is DOUBLE_BED (the intended object), but it spawned a chair (6) the out of sync configuration.

Considering define.dat code is just c++ code, it should be used as such, overriding all defines from confdef.h, to prevent double maintenance (that led to this out of sync unintended result).

So, Script/define.dat, during compilation, could be copied to Main/Include/define.h, overwritting it there. OR during compilation, Script/define.dat be compared to Main/Include/define.h, and fail, saying they are out of sync, if not equal, what is better to prevent losing work if you code it at Main/Include/define.h.

or am I wrong? :)

PS.: I will create a script to check both files to see the extension of the out of syncness.

AquariusPower commented 6 years ago

here is the sorted '#define' results from both files: confdef.h.log define.dat.log

When comparing them, you will see that there are a lot of identical entries. But all NEW entries are from define.dat.

And there are many entries out of sync (possibly causing unpredictable (now predictable) results): at define.dat

#define ANVIL 18
#define BIRCH 22
#define CACTUS 20
#define DOUBLE_BED 9
#define DWARF_BIRCH 24
#define FLESH_ID (4096 * 5)
#define GAS_ID (4096 * 3)
#define IRON_ALLOY_ID (4096 * 7)
#define LIQUID_ID (4096 * 4)
#define OAK 21
#define ORGANIC_ID (4096 * 2)
#define PALM 15
#define POOL_BORDER 13
#define POOL_CORNER 14
#define POWDER_ID (4096 * 6)
#define SHARD 19
#define SNOW_FIR 17
#define SNOW_PINE 16
#define SOLID_ID 4096
#define TEAK 23

at confdef.h (ugh.. the ones with << may be identical, so out of sync may be just half of it :)):

> #define ANVIL 12
> #define BIRCH 16
> #define CACTUS 14
> #define DOUBLE_BED 6
> #define DWARF_BIRCH 18
> #define FLESH_ID (5 << 12)
> #define GAS_ID (3 << 12)
> #define IRON_ALLOY_ID (7 << 12)
> #define LIQUID_ID (4 << 12)
> #define OAK 15
> #define ORGANIC_ID (2 << 12)
> #define PALM 9
> #define POOL_BORDER 7
> #define POOL_CORNER 8
> #define POWDER_ID (6 << 12)
> #define SHARD 13
> #define SNOW_FIR 11
> #define SNOW_PINE 10
> #define SOLID_ID (1 << 12)
> #define TEAK 17

linux commands used for reference:

cat ./Main/Include/confdef.h |egrep "#define *[A-Z_0-9]*" |grep -v "_CONFDEF_H_" |sort
cat ./Script/define.dat |egrep "#define *[A-Z_0-9]*" |sort
diff define.dat.log confdef.h.log |grep ">" |sed -r 's"[>] #define ([^ ]*) .*"\1"' |tr '\n' '|'
egrep "#define (ANVIL|BIRCH|CACTUS|DOUBLE_BED|DWARF_BIRCH|FLESH_ID|GAS_ID|IRON_ALLOY_ID|LIQUID_ID|OAK|ORGANIC_ID|PALM|POOL_BORDER|POOL_CORNER|POWDER_ID|SHARD|SNOW_FIR|SNOW_PINE|SOLID_ID|TEAK)" -w define.dat.log
AquariusPower commented 6 years ago

for ivandef.h most differences are non matching identifiers (the define name) what is ok. but some may be a problem like HEAD and TORSO (that have swapped values 1 and 2), compare it against define.dat.log cat ./Main/Include/ivandef.h |egrep "^#define *[A-Z_0-9]*" |egrep -v "_[^ ]*_H_$" |sort >ivandef.h.log ivandef.h.log

To remove all #defines from the .h files doesnt sound good tho... As the .dat ones are read, I think the best is to create a check to make it sure the values match. We may be able to use the "define to string" trick to match .dat with .h identifiers and values. Let me se what can be done..

ryfactor commented 6 years ago

Great, I didn't even realize 0.o

AquariusPower commented 6 years ago

I only detected in-game the problem of chair vs double bed, if we go one by one, checking what is being used directly from the .h files that mismatch the .dat ones, we may find more interesting or weird things happening in the game.

I am thinking on auto-generating c++ code to check if .dat is in sync with all defines from .h, I just don't know yet if we will be able to create a compilation error if out of sync, neither an abort message when running the game, only once.

So it may have to be a developer "2 steps" action: compile run (the code will be generated) compile again run (the check will be performed)

something like that I guess.

AquariusPower commented 6 years ago

created a validator that gave this "nice abort" result:

Defined LIGHT_GRAY with value 46518 from .dat file mismatches hardcoded c++ define value of 38066!
Defined PARASITE_MIND_WORM with value 2147483648 from .dat file mismatches hardcoded c++ define value of -2147483648!
Defined HEAD with value 1 from .dat file mismatches hardcoded c++ define value of 2!
Defined TORSO with value 2 from .dat file mismatches hardcoded c++ define value of 1!
Defined DOUBLE_BED with value 9 from .dat file mismatches hardcoded c++ define value of 6!
Defined POOL_BORDER with value 13 from .dat file mismatches hardcoded c++ define value of 7!
Defined POOL_CORNER with value 14 from .dat file mismatches hardcoded c++ define value of 8!
Defined PALM with value 15 from .dat file mismatches hardcoded c++ define value of 9!
Defined SNOW_PINE with value 16 from .dat file mismatches hardcoded c++ define value of 10!
Defined SNOW_FIR with value 17 from .dat file mismatches hardcoded c++ define value of 11!
Defined ANVIL with value 18 from .dat file mismatches hardcoded c++ define value of 12!
Defined SHARD with value 19 from .dat file mismatches hardcoded c++ define value of 13!
Defined CACTUS with value 20 from .dat file mismatches hardcoded c++ define value of 14!
Defined OAK with value 21 from .dat file mismatches hardcoded c++ define value of 15!
Defined BIRCH with value 22 from .dat file mismatches hardcoded c++ define value of 16!
Defined TEAK with value 23 from .dat file mismatches hardcoded c++ define value of 17!
Defined DWARF_BIRCH with value 24 from .dat file mismatches hardcoded c++ define value of 18!

I think it may be ok to fix all other values, but this one seems tricky (the value is assigned to a long): Main/Include/ivandef.h:158:#define PARASITE_MIND_WORM (1 << 31) // resulted in -2147483648 Script/define.dat:74:#define PARASITE_MIND_WORM (1 << 31) //////// restulted in 2147483648

we just need to check the define value in c++ code against others on the very code when fixing to match the .dat value to avoid other breaking conflicts.

AquariusPower commented 6 years ago

ok, made all the changes with as much care as I managed to understand. Tested wiz mode, saw

so from now on, changes made on define.dat or .h files, can be checked using a developer (user) in-game option to generate the validator and also validate. But the generated must be copied to src folder and recompiled and validated again if you make any changes to defines. I left some clear comments on the validator .h file to help understand the steps.

andrewtweber commented 6 years ago

I believe this can be closed based on the merged PR. Ping me if it needs reopening