search

07th-mod / ponscripter-fork

Fork of the Ponscripter visual novel engine to take advantage of SDL2 and improve Steam integration
GNU General Public License v2.0
15 stars 4 forks source link

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

Open uyjulian opened 2 years ago

uyjulian commented 2 years ago

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 commented 2 years ago

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, d43f6e0ed2a1181a3b6b660ab4cd3c074a308231, you can see clickWait has a similar if being added but with the check inverted, that's probably what it should have been

drojf commented 2 years ago

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 commented 2 years ago

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: https://github.com/07th-mod/ponscripter-fork/blob/cc7ab36eda35e6369a503b9f36007ec32422ee53/src/PonscripterLabel_text.cpp#L617-L700

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 commented 2 years ago

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

-    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 commented 2 years ago

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 commented 2 years ago

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?