Fixing text buffer texture/font corruption (#479)

[NQNStudios]

Debugging #479 has become a meandering process, with some other bugs needing to be fixed along the way. Might as well keep them in 1 PR.

• Text invisible in the message log. #479 demonstrates that the contents of the text buffer are NOT irrelevant for reproducing bugs. So I set up recording/replay for the burma shave easter egg. This also makes an easy way to mess with the buffer state when debugging (just mash &/*/&/*/&/* n times)
• When a replay throws an error, it puts up a showError() dialog. If the next action is a control_click, the system will try to click that control on the error dialog--which is totally divergent from the replay's intended behavior. So we should just stop replaying when an error happens.
• If you have a long replay and want to run it very fast, but then slow down when you get to the sequence that reproduces your bug, now you can add a <change_fps> to your replay to achieve that.
• Fixes for the 2 legacy replay errors that I opened yesterday

I still am quite stumped on the real error for #479. I asked for help in the SFML Discord channel, here's a link to that thread (you'll need a discord account and to join the SFML group to see it). But anyway, this link makes it easy for me to consult back to what people suggested.

EDIT: Changed the title to clarify one thing I do know about this bug: the text buffer array still contains the correct text. This is a case of the sf::Text that's rendered onto text_area_gworld being corrupt somehow. Maybe the font gets corrupted.

Fix #479
Fix #532
Fix #533

[NQNStudios]

@CelticMinstrel This bug was caused by this line:

62ddf3d#diff-0352715fc63f00d5823113f53777233ac00d70eb82df14dc28b2e9366710ee86L961

When creating a new PC, the empty slot was being filled by this cPlayer constructor which set the player name to "\n". Then, pick_pc_name() would be called, putting the "\n" in as the text field's default value.

Now, I don't know why, but it appears that the larger issue here is that if we accidentally try to render a newline character, the font object becomes borked. This probably explains #454 (one instance of which this PR also fixes) and maybe #417 as well. We need to find all the ways we might end up trying to do that... And maybe add an assertion that there are no newlines in strings being drawn.

In this case, setting the default player name to empty string, fixes #479 as reproduced by the log in that thread.

Was there any reason to set the player's default name to a newline and not an empty string?

[NQNStudios]

According to the blame, it looks like the "\n" default name has been in place since 16 years ago when Mac and Windows code were separate. I'm thinking its purpose is obsolete and an empty string is fine?

[CelticMinstrel]

According to the blame, it looks like the "\n" default name has been in place since 16 years ago when Mac and Windows code were separate. I'm thinking its purpose is obsolete and an empty string is fine?

I found the "\n" in the original source code released by Spiderweb Software - at party.c:528. I have no idea why it was there, but it used to be an sprintf call. If you used blame I'm guessing you saw this as well.

Now, I don't know why, but it appears that the larger issue here is that if we accidentally try to render a newline character, the font object becomes borked.

This strikes me as kind of weird. Are we doing something strange in our rendering? I don't think this happens in other projects where I use SFML – rendering a newline should just be a no-op. Though admittedly, other projects probably only render a newline as part of a run of text and not alone… or it's possible I had logic to split on newline so the newline actually isn't being rendered.

[NQNStudios]

I’ll dig into the nitty gritty of why it happens—we should probably know *exactly* why.

…

On Fri, Jan 17, 2025 at 8:43 AM Celtic Minstrel ***@***.***> wrote: According to the blame, it looks like the "\n" default name has been in place since 16 years ago when Mac and Windows code were separate. I'm thinking its purpose is obsolete and an empty string is fine? I found the "\n" in the original source code released by Spiderweb Software - at party.c:528. I have no idea why it was there, but it used to be an sprintf call. If you used blame I'm guessing you saw this as well. Now, I don't know why, but it appears that the larger issue here is that if we accidentally try to render a newline character, the font object becomes borked. This strikes me as kind of weird. Are we doing something strange in our rendering? I don't think this happens in other projects where I use SFML – rendering a newline should just be a no-op. Though admittedly, other projects probably only render a newline as part of a run of text and not alone… or it's possible I had logic to split on newline so the newline actually isn't being rendered. — Reply to this email directly, view it on GitHub <#534 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AATXBKKLSVZYQ3EJBY2STRT2LEJKJAVCNFSM6AAAAABVKHJUYKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOJYGUZDAMZUG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[NQNStudios]

I've modified draw_startup() to just directly do some tests.

void draw_startup(short but_type) {
	
        // We can change this line to demonstrate what breaks the normal text declared below
        sf::Text text("\n", *ResMgr::fonts.get("plain"), 12);


	mainPtr.draw(text);
	sf::Text text2("This text should appear normal", *ResMgr::fonts.get("plain"), 12);
	text2.setPosition(100, 100);
	mainPtr.draw(text2);
}

This first version, just putting "\n" in an sf::Text, doesn't break anything.

A newline in the middle of a string also doesn't break anything. Actually, SFML handles the newline.

sf::Text text("text\nwith a newline in it", *ResMgr::fonts.get("plain"), 12);

Now, using our cboe library functions:

	rectangle dest = { 0, 0, 50, 400};
	TextStyle style;	
	win_draw_string(mainPtr,dest,"\n",eTextMode::LEFT_TOP,style);

Still good.

win_draw_string with a newline in the middle:

Now I'm getting surprised this all still works.

#479 was caused by the newline being in the string for a cTextField, and those use draw_string_hilite so I tried that next:

	draw_string_sel(mainPtr,dest,"\n",style,{{0, 1}}, Colours::BLUE);
        // OR
        draw_string_hilite(mainPtr,dest,"\n",style,{{0, 1}}, Colours::BLUE);

both give the same result:

And still not reproducing the bug.

Still getting to the bottom of this.

[NQNStudios]

No more messing around, I tried just launching a text field with "\n" in it:

	static bool ran_dialog = false;
	if(!ran_dialog){
		cDialog pcPickName(*ResMgr::dialogs.get("pick-pc-name"), nullptr);
		pcPickName["name"].setText("\n");
		pcPickName["okay"].attachClickHandler([](cDialog& me, std::string _1, eKeyMod _2) -> bool {
			me.toast(true);
			return true;
		});
		
		pcPickName.run();
		ran_dialog = true;
	}

But this still didn't break anything!

[NQNStudios]

I still honestly don't know how to reproduce this bug without running the very long replay that entails printing to the buffer hundreds of times and doing all kinds of weird state changes. But even then it doesn't feel like it comes from a random memory leak, because Windows and Mac both share the problem (albeit presented differently visually).

[NQNStudios]

Could the newline in the default name have been put there to avoid a segfault/out-of-bounds here?

cboe/src/game/boe.party.cpp

Line 2225 in 3daf1d5

if(!isalpha(pcName[0])) {

[CelticMinstrel]

Could the newline in the default name have been put there to avoid a segfault/out-of-bounds here?

Seems plausible, but that wouldn't be a segfault even on an empty string, since strings need to be null-terminated (even std::string needs that so that c_str() works), and also because a short string doesn't allocate any memory (there's no heap pointer to dereference). It would probably be an out-of-bounds assertion in some debug C++ runtimes though.