MSG file structure mismatch

[AdamLacko]

Hey there,

I've been playing the CE and I managed to reproduce a crash that happens when the player takes damage during combat. Along with that, there are many formatting issues and apparent undefined behavior-related text outputs in combat messages in general.

I cloned the repo and traced the crash first. It's an exception thrown by snprintf due to format mismatch with the passed in arguments.

Upon further investigation, however, I compared the Combat.msg files from my distribution (v1.1, original retail CD) and the Steam distribution (according to the readme, v1.2). It turns out that the files have a different stucture and some strings have a different format.

Example: Constructing damage message

Fallout v1.1

{507}{}{were}
{508}{}{was}
{513}{}{%s %s hit for %d hit points}

Fallout v1.2

{513}{}{%s were hit for %d hit points}
{613}{}{%s was hit for %d hit points}
{713}{}{%s was hit for %d hit points}

The code in this repo is apparently written for v1.2 style of msg files, and trying to run v1.1 has led to either incorrect outputs (empty strings, memory garbage and other undefined behavior, etc.) or crashes due to wrong formats.

I fixed almost everything on my end and got it working for v1.1, but this issue begs for much more robust solution - version check and branched logic for both msg structures.

You may already know about this issue, but just in case you don't, I'll be glad to help to come up with solution to properly support different distributions with one binary.

I'd have to investigate other MSG files and see differences there, but my naive approach would be to try to isolate specific differences in the files (for instance, v1.1 style of Combat.msg doesn't have any IDs between 534 and 1110, while v1.2 adds variations of 500s into 600s and 700s, etc.) and branch the logic based on that.

[alexbatalov]

Hey Adam, migrating to v1.2 is one of the current goals, no wonder some code is already updated. However we should not ignore v1.1. I didn't find a way to query for current assets version from dat files themselves. I definitely don't want to go with configurable option (so please no version key in fallout.cfg or something like it). Do you have suggestions (doesn't have to pretty, "just works" is fine)? Checking v1.2 messages and fallback to v1.1 is simple. How about presence or order of formatting specifiers?

[AdamLacko]

I haven't found any versioning of these files either, so the determination has to be done otherwise. Indeed, no manual config nor version key, such a solution wouldn't even come to my mind. I have a suggestion - and an already working prototype of it. It's not pretty, but it should be error-proof.

I checked Fallout v1.0, v1.1 and Steam & GOG v1.2. First two share the same structure, in v1.2, the structure is changed as I described in the first post. That leads me to a conclusion that there are only these two types of structures that were ever released.

I wrote a function (below) that is called in combat_init() after the combat_message_file is loaded (on game startup).

int combat_get_message_file_type(MessageList* messageList)
{
    MessageListItem messageListItem;

    //query first item with ID within 600s - ID that does not exist in v1.1
    messageListItem.num = 608;
    if (message_search(messageList, &messageListItem)) {
        return COMBAT_MESSAGE_FILE_V1_2;
    } else
        return COMBAT_MESSAGE_FILE_V1_1;
}

What it does is that it tries to find an entry with ID 608 - the first ID that does not exist in v1.0/v1.1 and only exists in v1.2, since, apparently, the developers rewrote some of the combat message string construction logic - my guess is to better support localizations, for which, I assume, they added entries with IDs 600+ and 700+ and removed things like "was" and "were" being separate entries passed in as arguments and instead wrote these verbs right into those new entires, etc.

If the entry is found, the game uses v1.2 structure, if not, it's v1.0/v1.1.

Then, in combat_display() and combat_display_hit(), wherever the indices are set and substrings are created, the logic there is simply branched by

if (combat_message_file_type == COMBAT_MESSAGE_FILE_V1_1) {
...
} 
else {
...
}

where combat_message_file_type holds the type (int for now, in case we eventually find more than 2 types, but that's unlikely), and COMBAT_MESSAGE_FILE_V1_1 is a defined constant.

What do you think? Do you see any potential troubles I missed? Like I said, it's not pretty, it's primitive and quite crude to rely on ID presence like this, but I think it might just work. I already tested the combat_display_hit(), it works flawlessly for both game distributions. If you'll approve of this solution, I can rewrite the rest.