GitExl / WhackEd4

As a replacement for the DOS-based Dehacked, WhackEd4 allows you to load and edit Doom Dehacked files.
http://www.teamhellspawn.com/exl/whacked4
BSD 2-Clause "Simplified" License
42 stars 7 forks source link

Saving in 1.3.0b destroys DEHEXTRA state table of files made in 1.2.4 #20

Closed j0euk closed 2 years ago

j0euk commented 2 years ago

This just happened to me. An error in the Extended DeHackEd state definitions in 1.3.0-beta's cfg/basetable_extended.json means that saving a file made in 1.2.4 causes irreversible data loss in states 1089-3999, which must then be fixed by hand.

How to reproduce:

Here's a 'working' example. It uses the extended states to make the pistol fire 4 shots in a row (bit of a contrived example, but good enough to demonstrate the bug). It also makes the BFG projectile red by setting its TRANSLATION and TRANSLATION2 flags, to demonstrate a tangentially related bug that I'll be covering in a moment.

Save this as a .bex file:

Patch File for DeHackEd v3.0
# Created with WhackEd4 1.2.4 BETA
# Note: Use the pound sign ('#') to start comment lines.

Doom version = 21
Patch format = 6

Thing 36 (BFG projectile)
Bits = NOBLOCKMAP+TRANSLUCENT+NOGRAVITY+TRANSLATION+DROPOFF+MISSILE+TRANSLATION2

Frame 13
Duration = 1
Next frame = 1089

Frame 1089
Sprite number = 3
Duration = 1
Next frame = 1090

Frame 1090
Sprite number = 3
Duration = 1
Next frame = 1091

Frame 1091
Sprite number = 3
Duration = 1
Next frame = 14

[CODEPTR]
FRAME 1089 = FirePistol
FRAME 1090 = FirePistol
FRAME 1091 = FirePistol

Now, open the file in WhackEd 1.3.0-beta and save without doing anything else. The .bex file now looks like this:

Patch File for DeHackEd v3.0
# Created with WhackEd4 1.3.0 BETA
# Note: Use the pound sign ('#') to start comment lines.

Doom version = 21
Patch format = 6

Frame 13
Duration = 1
Next frame = 1089

Frame 1089
Sprite number = 3
Duration = 1

Frame 1090
Sprite number = 3
Duration = 1

Frame 1091
Sprite number = 3
Duration = 1
Next frame = 14

[CODEPTR]
FRAME 1089 = FirePistol
FRAME 1090 = FirePistol
FRAME 1091 = FirePistol

When testing this newly saved .bex file in PrBoom+ 2.6.1um, the pistol now fires constantly in an infinite loop, even into the minus figures, and cannot be switched away from.

The bug is caused by an error introduced in the state table definitions. In WhackEd 1.2.4, the default Next State for all extended states is the same state (e.g. 1089's Next State is 1089), as it is in source ports. In WhackEd 1.3.0b, this is changed to the default Next State being the subsequent state (so 1089's Next State is now 1090). For any states which defined this in a .bex file, WhackEd 1.3.0 assumes they are unchanged from the default and doesn't include the Next State line for those states when saving, resulting in data loss, incorrect behaviour in source ports, and wasted evenings trawling through a ruined state table and fixing any that were messed up. Of course I pressed Save twice so my .bex.bak file is also wrecked.

Fixing involves some changes to cfg/basetable_extended.json to restore the old correct behaviour; I will submit a pull request to this end as soon as I have figured out this 'personal access token' nonsense that GitHub has decided to foist on us (FFS, can nothing be easy these days?).

As a side note, the cfg/basetable_mbf.json that Extended DeHackEd inherits from does not include the TRANSLATION2 flag defined in that port, resulting in the following error when opening the above .bex file in 1.3.0-beta:

Exceptions occurred during loading. The patch may be corrupted.

Last exception:
Ignoring unknown thing flag key "TRANSLATION2".

It also means the red colour of the BFG projectile is lost on save.

I fixed this locally by adding an entry for TRANSLATION2 to the thingFlags table in cfg/basetable_mbf.json, but it has added it underneath the UNUSED1 flag used by Boom (which cfg/basetable_mbf.json inherits from). If it's possible to 'remove' thing flag options set by inherited basetables, let me know how and I can submit a PR for this too.

brokenstate.bexs+fixes.zip

GitExl commented 2 years ago

Glad you caught this before I released a final version! I've merged the fix and also added the TRANSLATION2 flag for the MFG config together with the ability for thing flags to override existing ones. Thanks!

hawkwind3 commented 2 years ago

TRANSLATION2 should also be in basetable_boom.json like this ...

"TRANSLATION2": { "index": 27, "name": "Color 2 (brown\red)", "description": "This thing's green colors will be mapped to brown (or red if the Color 1 flag is also set)." }, "UNUSED1": { "index": 28, "name": "Unused", "description": "Unused." }, "UNUSED2": { "index": 29, "name": "Unused", "description": "Unused." }, "UNUSED3": { "index": 30, "name": "Unused", "description": "Unused." }, "TRANSLUCENT": { "index": 31, "name": "Translucent", "description": "Apply translucency." } },

j0euk commented 2 years ago

Cheers for merging the pull request (and apologies for delayed response). I could've been a bit less grumpy in my initial post. That was what I got for using software with a big warning that says 'this is beta software, expect bugs!' to work on my only copy of a project. It only took around half an evening to fix the frame table in the end, so it really wasn't warranted to make such a fuss. Thanks again for all your work, I continue to use and love WhackEd.

GitExl commented 2 years ago

No problem, running into data destroying bugs is always painful. But you provided very detailed reproduction instructions and even a fix, so thanks again!