Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[linux] Combat messages show %s character #162

Open
odsquad64 opened this issue Apr 27, 2024 · 6 comments · May be fixed by #187
Open

[linux] Combat messages show %s character #162

odsquad64 opened this issue Apr 27, 2024 · 6 comments · May be fixed by #187

Comments

@odsquad64
Copy link

When attacking the cave rats at the beginning of the game it shows the message "She looks: %s. Unhurt." It seems like "Unhurt" is supposed to replace the "%s" there. After attacking the rats once it changes to "She looks: %s. Wounded." Attacking some lesser mole rats later on I get the same message. I've searched but I haven't had any luck finding any mention of anyone else experiencing this issue.

Screenshot_2024-04-27_00-06-33

@GBPlayer88
Copy link

Can confirm the bug in Windows too.

@trevorknight
Copy link

And MacOS

@dje4321
Copy link

dje4321 commented May 21, 2024

From my PR #185 there were atleast 2 snprintf calls that did not provide the format specifier. I wonder if there are more missing/incorrect specifiers in the code

@hippydave
Copy link

From my PR #185 there were atleast 2 snprintf calls that did not provide the format specifier. I wonder if there are more missing/incorrect specifiers in the code

Thanks! From a quick search for sprintf, I think obj_examine_func is the relevant function.

@dje4321
Copy link

dje4321 commented May 21, 2024

Ive found part of the issue and have confirmed a partial patch to resolve it.

at src/game/protinst.cc:423 there is this line of code
snprintf(formattedText, sizeof(formattedText), v66.text, v63.text, hpMessageListItem.text);
v66.text contains "%s %s."
v63.text contains "He Looks: %s."
hpMessageListItem.text contains "Unhurt"

It appears the code author is trying to use v66 as a recursive format specifier for v63 and hpMessageListItem but v63 already contains the needed format specifier for hpMessageListItem to be used properly. Just removing v66 format and allowing v63 to be used in its place seems to resolve the issue with my limited testing.

Pic from GDB confirming the issue
Screenshot from 2024-05-21 13-23-57

Ill probably submit a PR to fix this issue at some point once I can get a good look over the function to try and find any other locations that could be wrong. However this codebase is absolute ass to (look at/work on) with the most non-descript names imaginable so dont hold your breath

dje4321 added a commit to dje4321/fallout1-ce that referenced this issue May 21, 2024
obj_examine_func():408-424 has a redundant format specifier. v66 provides a "%s %s" format specifier for objects v63 && hpMessageListItem,
however v63 already provides a format specifier in the form of "GENDER looks: %s" for use by hpMessageListItem. Technically the most correct
fix is to use v63 to create a secondary format specifier like how src/game/protinst.cc:314 does it, However it is my belief that this will
cause a regression if the target is not crippled because MSG_ID 521 is "%s %s." which will append a secondary period onto the end of the string.

Modifed obj_examine_func() comments for added clarity
dje4321 added a commit to dje4321/fallout1-ce that referenced this issue May 21, 2024
obj_examine_func():408-424 has a redundant format specifier. v66 provides a "%s %s" format specifier for objects v63 && hpMessageListItem,
however v63 already provides a format specifier in the form of "GENDER looks: %s" for use by hpMessageListItem. Technically the most correct
fix is to use v63 to create a secondary format specifier like how src/game/protinst.cc:314 does it, However it is my belief that this will
cause a regression if the target is not crippled because MSG_ID 521 is "%s %s." which will append a secondary period onto the end of the string.

Modifed obj_examine_func() comments for added clarity
@dje4321
Copy link

dje4321 commented May 21, 2024

I have merged this PR (and several others for bug fixes) into my git repo for those who are interested. You will be required to compile it yourself as I dont want to setup github CI for this

https://github.com/dje4321/fallout1-ce/tree/PR-Merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants