Fix page breaks not working properly when any of the 'lang' series of commands are not used

[uyjulian]

Add a flag to be set when any of the 'lang' series of commands are used e.g. langen, langjp, langall.
Only check current_read_language on page wait if current_language_set is set.

Fixes page breaks not working properly when any of the 'lang' series of commands are not used.

[TellowKrinkle]

If I'm reading the original code correctly, the initial state is the same as if you had run langall, so while I don't know exactly why that if statement is there, I don't think it's what you want to modify

[TellowKrinkle]

This looks like "if current_read_language is not the display language" (AKA "if the script's text is not supposed to be shown")

[uyjulian]

The initial state is when langall is set (so current_read_language would be -1).

However, when the 'lang' series of commands are never used, current_read_language would be stuck at -1, so clickstr_state = CLICK_NEWPAGE; never gets executed, resulting in the page break glyph not being shown and text continually printing off screen.

This if statement is the correct one to modify.

[TellowKrinkle]

Maybe the actual issue is that the if statement is inverted from what it should be? If you look at the commit that added it, d43f6e0, you can see clickWait has a similar if being added but with the check inverted, that's probably what it should have been

[drojf]

I have a question about this...in the commit d43f6e0 you posted, and the current version of the code, the ! and # handling has the same condition in the if statement:

    // Chronotrig, multilang inline command hack, ignore ! and # commands when current language isn't being read
    if (current_read_language != -1 && current_read_language != current_language) {
       ...
       //! and # handling goes here>
       ...
    }

Say you set the language to English, then current_read_language would be set to 0, so the first part of the if condition would be true. That would mean that the ! and # handling would run if current_read_language != current_language is true, that is if the language is not the current language.

I don't remember this being broken in the executable, so I guess I'm misunderstanding something here? Or do ! and # commands really only run on the opposite language that is selected?

[drojf]

Never mind, it looks like it's working as intended (I did a quick test and the ! handling works properly) - it's meant to execute on the opposite language, the actual handling for ! and # occurs here:

ponscripter-fork/src/PonscripterLabel_text.cpp

Lines 617 to 700 in cc7ab36

	else if (ch == '!') {
	++string_buffer_offset;
	if (script_h.readStrBuf(string_buffer_offset) == 's') {
	++string_buffer_offset;
	bool in_skip = (skip_flag || ctrl_pressed_status);
	int prev_t = sentence_font.wait_time;
	if (script_h.readStrBuf(string_buffer_offset) == 'd') {
	sentence_font.wait_time = -1;
	++string_buffer_offset;
	}
	else {
	int t = 0;
	while (script_h.isadigit(script_h.readStrBuf(string_buffer_offset))) {
	t = t * 10 + script_h.readStrBuf(string_buffer_offset) -'0';
	++string_buffer_offset;
	}
	sentence_font.wait_time = t;
	while (script_h.isawspace(script_h.readStrBuf(string_buffer_offset)))
	++string_buffer_offset;
	}
	if (!in_skip && (prev_t == 0) && (sentence_font.wait_time != 0))
	flush(refreshMode());
	}
	else if (script_h.readStrBuf(string_buffer_offset) == 'w'
	|| script_h.readStrBuf(string_buffer_offset) == 'd') {
	bool flag = false;
	bool in_skip = (skip_flag || draw_one_page_flag || ctrl_pressed_status);
	if (script_h.readStrBuf(string_buffer_offset) == 'd')
	flag = true;
	++string_buffer_offset;
	int tmp_string_buffer_offset = string_buffer_offset;
	int t = 0;
	while (script_h.isadigit(script_h.readStrBuf(string_buffer_offset))) {
	t = t * 10 + script_h.readStrBuf(string_buffer_offset) - '0';
	++string_buffer_offset;
	}
	while (script_h.isawspace(script_h.readStrBuf(string_buffer_offset)))
	string_buffer_offset++;
	flush(refreshMode());
	if (flag && in_skip) {
	skip_to_wait = 0;
	return RET_CONTINUE | RET_NOREAD;
	}
	else {
	if (!flag && in_skip) {
	//Mion: instead of skipping entirely, let's do a shortened wait (safer)
	if (t > 100) {
	t = t / 10;
	} else if (t > 10) {
	t = 10;
	}
	}
	skip_to_wait = 0;
	event_mode = WAIT_SLEEP_MODE;
	if (flag) event_mode |= WAIT_INPUT_MODE;
	key_pressed_flag = false;
	startTimer(t);
	string_buffer_offset = tmp_string_buffer_offset - 2;
	return RET_WAIT | RET_NOREAD;
	}
	}
	else {
	--string_buffer_offset;
	goto notacommand;
	}
	return RET_CONTINUE | RET_NOREAD;
	}
	else if (ch == '#') {
	char hexchecker;
	for (int i = 0; i <= 5; ++i) {
	hexchecker = script_h.readStrBuf(string_buffer_offset + i + 1);
	if (!script_h.isaxdigit(hexchecker))
	goto notacommand;
	}
	sentence_font.color
	= readColour(script_h.getStrBuf(string_buffer_offset));
	string_buffer_offset += 7;
	return RET_CONTINUE | RET_NOREAD;
	}

I think what happens is that code deliberately runs on the opposite language to eat all the tokens before it can get to the "real" ! and # handling logic.

[drojf]

Just to be clear, I think this is the change tellow is suggesting: 9e72186

-    if (current_read_language != -1 && current_read_language != current_language) {
+    if (current_read_language == -1 || current_read_language == current_language) {
         clickstr_state = CLICK_NEWPAGE;
     }

That is, it will trigger if either langall mode is on, or the current text belongs to the currently selected language, matching the logic of the CLICK_WAIT part.

[uyjulian]

I believe that changing the current_read_language != -1 && current_read_language != current_language logic may possibly break some existing behavior, so that is why I decided to add a separate variable that is set on using the 'lang' series of commands in order to avoid breaking existing behavior.

Because of the multiple layers of control flow occurring, I'm not too sure on a solution without a separate variable that does not break existing behavior…

[TellowKrinkle]

I believe that changing the current_read_language != -1 && current_read_language != current_language logic may possibly break some existing behavior

What do you expect it to break?

• Scripts where someone put random page breaks in langall sections on purpose because they knew it would be a no-op? If anyone actually did this, too bad for them
• Scripts that had page breaks in langall sections and no one noticed they weren't working? Then I would consider this fixing the problem, not breaking anything
• Scripts where someone purposely put page breaks in the opposite language only? Seems incredibly unlikely, and if someone actually did this instead of fixing the engine, too bad for them
• Scripts where someone put page breaks in one language only and didn't notice that they were affecting the wrong language? Bugfix