07th-mod / umineko-question

94 stars 9 forks source link

Hard crash when executing `gosub *turu_pettann` #145

Open drojf opened 4 years ago

drojf commented 4 years ago

We had a reported hard crash when the gosub *turu_pettann subroutine was executed (it displays lots of scrolling text across the screen nicovideo style.

Upon commenting out that line (which only occurs once), the problem was fixed.

I think this has been reported once before. As it might be a random/system specific crash, my only action for now is to open this issue to keep tabs on it.

drojf commented 4 years ago

Another user reported the same error:

Hi, I am playing Umi for the first time, I get this error immediately after this CG appears (beginning of ep 2) right when the first text over this CG is about to appear. I am playing with default settings except I am using the original sprites. Installed with latest automatic installer.

Is there some fix or should I just try reinstalling entirely?

I think the user is on Windows 7 (judging from their screenshot).

A re-install was performed, but it didn't fix the problem. I requested some more information from the user:

As I went to get the stderr and stdout, it randomly worked and the scrolling text displayed as intended. I saved after that since I wanted to make sure I was able to progress. I should have saved the stderr and stdout from this session since it worked, but I didn't since I thought the problem had fixed itself somehow. After I verified my save past that point worked, I went back to try again, and this time it crashed yet again. I will upload the stderr and stdout for this session. So it is a fickle problem, perhaps something to do with graphical capabilities since I am running on a laptop with integrated graphics.

That scene spawns many text sprites which all simultaneously scroll across the screen. I would suspect on lower end systems/older systems perhaps it cannot handle that and causes a crash in some cases.

Here are the stdout, stderr, and save files from the user: erik_error_report.zip

The stderr has a Warning: UTF8Encoding::Decode called on incomplete character (string: �), but I think that's unrelated. I can't spot any obvious errors in the logs, and I haven't tested the save files yet.

EDIT:

To attempt to reproduce the crash, I did a quick test by running the game on a Windows 10 Netbook ( Intel Celeron N3450/Intel HD 500). I ran the game 3 times on that scene using the provided save file but it didn't crash.

drojf commented 2 years ago

We're still getting reports of this crash. Since the actual part that crashes is only the niconico style text overlay (everything else will display as normal), it's probably worth adding a toggle or something to disable it so people can continue playing it crashes. I never realized it but that part is only on the screen for a couple seconds, the rest of the sequence is not animated

Another way is to just put instructions on the wiki to tell you to comment out the line: gosub *turu_pettann

A workaround would be to hard bake the sprites into an image, then just scroll it across the screen.

Another workaround if we wanted to copy the PS3 version is to ... play the PS3 video that plays in the PS3 game, however since we already have the option in the installer to make the game similar to the original game, we'd probably still want to support the original game's version somehow.

Reference video for that scene in the old Umitweak version: https://www.youtube.com/watch?v=whd4HF9GWEs&t=10438s

TellowKrinkle commented 2 years ago

Ran it with asan and it reported a buffer overflow, will inspect

drojf commented 2 years ago

This is more relating to the new .exe than this issue (but the bug(s) might be in both). We had some reports of crashes in the answer arcs (for that user who's testing the new .exe on their really old laptop).

I was thinking of testing the new .exe + answer arcs by just running the game in auto mode overnight while running a monitoring program.

How hard would it be to get .exes with address sanitizer compiled in for Windows (for me to test at least)? Can we just distribute those versions for testing instead (is there a big performance hit?)? Do you have any other things/recommendations I can use to test for errors?

TellowKrinkle commented 2 years ago

Okay it appears it's the line lsph %tmp,":s#80ff00  It's started━━━(゚∀゚)━━━!!!! ",0,0 : inc %tmp, the engine doesn't like the trailing space

How hard would it be to get .exes with address sanitizer compiled in for Windows

No clue how to compile with asan on Windows. I can try adding -fsanitize=address to the build but it may not work

TellowKrinkle commented 2 years ago

Okay should be fixed by https://github.com/07th-mod/ponscripter-fork/commit/5fe5b0dc2680f894aa4f2e712678cb9235c1f64e

TellowKrinkle commented 2 years ago

Side note, the non-fullwidth spaces are removed by the engine, if you want to keep them you can put a ^ or just use fullwidth spaces

drojf commented 2 years ago

Would this be enough to fix it on the old .exe?

https://github.com/07th-mod/umineko-question/commit/1c66e576a60e1c0796e524353ef92d3a3252be76

I don't think it makes a difference so I just removed the space instead of replacing it with a full-width space.

TellowKrinkle commented 2 years ago

Yep should be fine Spaces at the end shouldn't do anything anyways It's the spaces at the beginning and in the middle that will be skipped by https://github.com/07th-mod/ponscripter-fork/blob/5fe5b0dc2680f894aa4f2e712678cb9235c1f64e/src/PonscripterLabel_text.cpp#L222

TellowKrinkle commented 2 years ago

e.g. in that line in the beginning, there's currently one fullwidth space followed by four normal spaces So think the engine would render  It'sstarted━━━(゚∀゚)━━━!!!! instead

TellowKrinkle commented 2 years ago

BTW -fsanitize=address didn't work: https://github.com/07th-mod/ponscripter-fork/runs/3453169842

If you have anything else you want to try you can mess around on that branch

drojf commented 2 years ago

OK, I'll see what I can do.

I just checked in-game and you're right, the spaces don't appear, so I might as well fix it and use full width spaces between words.

TellowKrinkle commented 2 years ago

You can also just prefix the string with ^, which will let you use normal size spaces Fullwidth spaces are rather large, I think "It's started" would look a bit weird (Prefix as in after the color code, before the rest of everything)

drojf commented 2 years ago

The user confirmed this fixed the issue for them (removing the space at the end)

drojf commented 2 years ago

While changing the script did fix the crash on the old .exe, it seems to cause some severe audio corruption issue for this particular user.

How it's supposed to sound: festival_and_next_scene.zip

LOWER YOUR AUDIO VOLUME - these contain full loudness raw memory played as audio (I'm guessing): distorted audio samples.zip I think it's the sound effect which is corrupted, as you can still hear the voices playing properly in the background.

Since the issue persisted even when not using a save file, I assume I've either missed some places where it causes the problem, or there is some other memory issue there (or this issue is specific to this user).

EDIT: forgot to note that the user did not have the option applied which changes the song back to the original, and changes the CG back to the original.