WeiDUorg / weidu

WeiDU is a program used to develop, distribute and install modifications for games based on the Infinity Engine.
http://www.weidu.org
GNU General Public License v2.0
90 stars 20 forks source link

--parse-check tra refs #178

Open burner1024 opened 4 years ago

burner1024 commented 4 years ago

Thank you for implementing this option. I made some tests, and there's one issue that makes it a bit awkward to use. Many D/BAF contain tra references, and obviously those aren't loaded in parse-only mode, however apparently parser expects them to be, and returns failure. (And for some reason, tp2/tpa with tra refs work fine).

weidu --game ~/bg2 --debug-assign --no-exit-pause --noautoupdate --parse-check baf shaper_kit/baf/wm_chaos.baf 
[weidu] WeiDU version 24700
[/home/user/bg2/chitin.key] 436 BIFFs, 68932 resources
SET %SAVE_DIRECTORY% = ~./save~
SET %MPSAVE_DIRECTORY% = ~./mpsave~
SET %USER_DIRECTORY% = ~.~
[/home/user/bg2/dialog.tlk] 462663 string entries
[/home/user/bg2/dialogf.tlk] 462663 string entries

[/home/user/bg2/DATA/G_BIFF2_3.BIF] 29968658 bytes, 2196 files, 0 tilesets
[/home/user/bg2/DATA/TB#GEN4.BIF] 25685731 bytes, 1005 files, 2 tilesets

[shaper_kit/baf/wm_chaos.baf] PARSE ERROR at line 36 column 1-31
Near Text: @10201
    unable to resolve translation string @10201

[shaper_kit/baf/wm_chaos.baf]  ERROR at line 36 column 1-31
Near Text: @10201
    Parsing.Parse_error
ERROR: parsing [shaper_kit/baf/wm_chaos.baf]: Parsing.Parse_error
ERROR: File [shaper_kit/baf/wm_chaos.baf] was NOT successfully parsed as type [baf]

FATAL ERROR: Parsing.Parse_error

I think it's safe to just skip tra checking in parse mode?

FredrikLindgren commented 4 years ago

The BAF parser is a bit particular in that it raises a parse error unless resolving strings return the expected result. You have to kick it just right to get it to stop. The D parser should not have exhibited this behaviour. Do you have an example?

Regardless, this should be fixed in the next version.

burner1024 commented 4 years ago

No, I just assumed they behave the same.

D parser indeed skips trarefs. It does output warnings on missing action ids (for example, if used with --nogame), which still results in "ok" status.

ERROR locating resource for 'get_ids_map'
Resource [ACTION.IDS] not found in KEY file:
    [ -- NO GAME -- /chitin.key]

[action list near line 480, column 42 of shaper_kit/dialog/wm_level.d] PARSE WARNING at line 480 column 1-28
Near Text: )
    [AddSpecialAbility] not found in ACTION.IDS
WARNING: cannot verify action ~AddSpecialAbility("SPCL917")~: Parsing.Parse_error

ERROR locating resource for 'get_ids_map'
Resource [ACTION.IDS] not found in KEY file:
    [ -- NO GAME -- /chitin.key]

[action list near line 485, column 42 of shaper_kit/dialog/wm_level.d] PARSE WARNING at line 485 column 1-28
Near Text: )
    [AddSpecialAbility] not found in ACTION.IDS
WARNING: cannot verify action ~AddSpecialAbility("SPCL913")~: Parsing.Parse_error
File [shaper_kit/dialog/wm_level.d] was successfully parsed as type [d]

I'm not sure what's the best way to handle this, as mods can append to trigger/action.ids (tobex and stuff). On the other hand, it's rare. Maybe just leave it as is.

FredrikLindgren commented 4 years ago

The --nogame thing is more bothersome, since WeiDU loads no IDS files with --nogame, but the baf parser needs IDS files in order to successfully convert a BAF file into WeiDU's internal representation, and won't report success unless the BAF text can be successfully converted. D, on the other hand, needs not convert actions and triggers, since DLG files do not store these as byte code.

burner1024 commented 4 years ago

Why does D report ERROR as above, then?

FredrikLindgren commented 4 years ago

Oh, that was because the D parser has some optional bits for verifying actions and triggers. It's been turned off for --parse-check in commit 6859343e82c13dbe27c2f952085608dc416bdbf6. The problem with BAF and --nogame, however, is that WeiDU does not have enough information to successfully parse BAF files without ACTION.IDS and TRIGGER.IDS, which are not loaded with --nogame. D files can be parsed with --nogame, because action and trigger strings can be completely opaque without affecting parsing.

burner1024 commented 4 years ago

Oh, that was because the D parser has some optional bits for verifying actions and triggers. It's been turned off for --parse-check in commit 6859343.

Ah, ok, I thought it only applied to trarefs. I just expect the parser to be consistent - if it says ERROR or WARNING, then a non-zero exit code must follow.

The problem with BAF and --nogame, however, is that WeiDU does not have enough information to successfully parse BAF files without ACTION.IDS and TRIGGER.IDS, which are not loaded with --nogame. D files can be parsed with --nogame, because action and trigger strings can be completely opaque without affecting parsing.

Yes, of course, I realize that. It's not that much of a problem per se, as I can just display a message to the user, requiring to enter a path to the game in the setting. Where that fails is when a mods applies its own entries to IDS and then uses them in the scripts, but I assume most mods don't and its better to pospone thinking about that until modders actually come complaining.