07th-mod / higurashi-assembly

9 stars 8 forks source link

Sprites stuck to the screen #45

Open ghost opened 5 years ago

ghost commented 5 years ago

This is a long-standing bug that we've been aware of ever since Tsumihoroboshi was released last year - however, I have had nearly no success replicating the bug but I have some ideas now and I figured it's good to document this as well.

The bug: Sprites will stick to the screen, even persisting through a quit to the main menu. Someone has to reload the game and - possibly, not sure - use chapter jump instead of reloading their save to get around the issue. According to one user's report too, when attempting to switch sprite sets after this, that gets broken too, like characters get swapped. Arcs affected: At least Meakashi and onward conditions: not well known, but people report clicking through the text fast, some even using CTRL to skip.

I was able to replicate this once. One characters stuck to the screen, but the game uses 2 different scenes to facilitate screen transitions; the sprite would get stuck to only one scene. Haven't been able to replicate since then. I disabled some checks in the lipsync code for whether the animation was still active and was able to reproduce this behavior much more easily.

There is a known possible race condition in the lipsync implementation where at the point when an animation is in progress but the user clicks forward in the text, the game state is updated to cancel that animation, but the code will still try to return that character's expression to their mouth closed if it's currently open. The lipsync code is running on a coroutine and the race condition is that the game can continue executing other code once the user clicks forward in the text. But if this is indeed a lipsync related problem, then it's surprising we have never had any reports about this issue in the question arcs. Further clouding the issue is that almost all our players use lipsync, which might be signaling us in the wrong direction, but the fact that disabling other lipsync-related guards allowed me to more freely reproduce the issue makes it pretty likely.

Theory: The scene transitions may be critical here. If a character has some spoken line, and the player cuts it off early, and the next line in the script is to transition the background, then it may cause the closed-mouth sprite to be drawn onto the other scene.

Possible solution: Whether that's correct or not, a great step would be to eliminate that race condition. However, the game engine seems to have only fixed-length-of-time wait objects with different labels. We can add a WaitForLipsync wait type, and set the wait time to be the same as the corresponding voice time, possibly with a safety buffer, but that still isn't very clean. We may be able to instead add a more general WaitForCoroutine wait type, which will only clear when that coroutine actually terminates.

Still unclear on why this only seems to happen in Kai, though there were plenty of changes going into Meakashi, so it's not entirely surprising.

ghost commented 5 years ago

Attempted a fix and sent it to some people for testing / review. The change is to invalidate all lipsync coroutines that may be currently running when active scenes are swapped during background/scene transitions. This is more strict than disabling, which allows the character's expression to return to their mouth closed to "finish" the process. Branch is https://github.com/07th-mod/higurashi-assembly/tree/f-sticky-sprite-fix

drojf commented 4 years ago

Here's a save file from matsu with the bug - "The 2 Hanyuu saves don't appear to reproduce the issue when loaded, but the Mion one did for me."

Saves Download Link: ch8saves.zip

Do you guys still want save files? I'll keep compiling them here unless they're not useful.

Neptop_2020_08_16_766682644_small Neptop_2020_08_16_871295074_small

drojf commented 3 years ago

Recently we found an issue where FadeAllBustshots() does not actually clear sprites on layer 0, which led to some situations similar to this issue where sprites would get stuck, but they were repeatable.

You can see that discussion here: https://github.com/07th-mod/tsumihoroboshi/issues/47

After some investigation, it does seem that the random sprite sticking issue is also due to using layer 0 to draw sprites. We only use layer 0 to draw sprites in the answer arcs, which is also where this random sprite issue happens. The screenshots we get also seem to correlate to where layer 0 is used.

There was some discussion in discord just now related to this and I thought I should record it here. Some paats of the conversation have been omitted for clarity

OP

I'm not sure if this was already fixed, but I just experienced this bug while playing through Matsuribayashi: https://cdn.discordapp.com/attachments/392489108875771906/775641596015411210/SPOILER_unknown.png When Keiichi comes to speak a few lines before, he appears behind Hanyuu and Hanyuu stays there.

O[A]

I'm not sure if this is fixed either, orian found something related to this but this bug has been going on for a while. Restarting the game should help

OP

I also experienced something similar with Mion during a BG transition a few lines later, but reloading a save and getting there again didn't trigger the bug. Weird.

orian:

but it can be circumvented through some script changes so I 'fixed' that scene now there shouldn't be that issue again

drojf:

wait, how do you do that? Can't I just make the DLL always do it? you're talking about this issue OP just posted right? ah I'll just look at what you did on github edit: https://github.com/07th-mod/matsuribayashi/commit/24bd81a6dca50226d9893d41e5b2a2ba4c9aa2b8 are you sure it's the layer 0 thing? I thought we never confirmed it? is the stuck sprite in this case matching where layer 0 is used?

(paraphrased)

you have numbered layers, but their depth is another value 0 is associated with 9, but 1 is 0, 2 is 10 and 3 is 20

The problem started when MG started using layer 0 they changed layer 1 to use priority 8 I changed that cause unlike layer 0, layer 1 was widely used already and it had to be kept consistent but yeah, there's no doubt the layer 0 is seemingly causing it to happen semi consistently (enough to have a lot of reports for the exact scene and sprites) while the previous layering issue was more vague and random

hopefully by changing those cases it should reduce the issue happening at all, until we can figure out the root

drojf commented 3 years ago

It seems that this bug can occur even on sprites without lipsync (unless lipsync still runs on sprites without lipsync) as evidenced by this image from minagoroshi:

Skarmbild_1752

It has also been seen on ryukishi sprites too according to doctordiablo, but I don't have a screenshot for that at the moment

I also recorded another instance for minagoroshi here: https://github.com/07th-mod/minagoroshi/issues/17

https://user-images.githubusercontent.com/1249449/79951675-77644700-84bc-11ea-8dea-a530a7811020.jpg

drojf commented 3 years ago

A couple days ago I investigated this issue, and think I made some headway. Even if it doesn't turn out to fix anything, this can be a record of what I looked into.

tldr; I managed to reproduce the issue, attempted a fix which involves replacing the custom ModDrawBustShot code with a straight texture swap, and sent it to a tester for testing (doesn't seem to cause any additional bugs so far at least).

For the details, see below;


I had a report recently here: https://github.com/07th-mod/tsumihoroboshi/issues/37#issuecomment-921480304 which had already been reported before.

I asked the user if they liked clicking through text /voice before it had finished, and they said yes (relating to your initial comment of the stucksprite bug).

I then tried to reproduce it on my computer, and found I could reproduce it quite consistently (maybe every 1/6 times). I just clicked really fast so that the lipsync got cancelled around when the fadeout of all sprites happened (at least I think that's what is happening). I recorded a video as well, but I can just reproduce it if I want to.

I went back and read idealpersona's initial comment in this thread.

and what I experienced did seem to coincide with your comment - probably the lipsync function has some race condition where the final ModDrawBustshot is called just after the fadeout command is called, causing the sprite to stay stuck to the screen. I still don't fully understand how the game's layer system works, but after playing around with it I'm convinced that if you do anything out of the ordinary it will break unexpectedly.

I'm pretty sure that the whole way lipsync is done (by calling a custom version of moddrawbustshot without adding it to the 'list of actions' to be performed, at an unpredictable time to the 'main' game logic) is what is causing this problem.


To bypass this completely, the new method doesn't call any variant of moddrawbustshot at all - it just changes the texture of the layer of the character (literally GetLayer(layer)?.SetPrimaryTexture(tex2d). This way, it doesn't interfere with any bustshot drawing or fading, even if called at the wrong time.

The worst that could happen is that the wrong expression would be displayed (or the wrong character, if the same layer is reused for a different character right away). This wrong expression/sprite would also only persist until the next time the layer is redrawn, so it would go away very quickly. This does seem to fix this instance, but I haven't spent a huge amount of time trying to reproduce it. See this commit: https://github.com/07th-mod/higurashi-assembly/commit/fe0423ac388ed96beccb4276dd7c51d4d1269ab5


I also realize (as you said in your initial comment) that it may not be related to lipsync, but I'll try this fix and see how it goes.


More thoughts: I did a git blame on the moddrawbustshot/moddrawlayer command, as I wanted to know who made/changed it. This function was basically never changed from the initial commit on git. I think it was just a copy of the normal moddrawbusthot/moddrawlayer from Onikakushi.

This might explain why the bug only happens in chapters 6-8 - because we're using the moddrawbustshot/moddrawlayer command from chapter 1 on chapters 6-8. If any changes were made in 6-8, they wouldn't have been mirrored back into the lipsync's unique copy of moddrawbustshot/moddrawlayer.

You can see these functions in https://github.com/07th-mod/higurashi-assembly/commit/fe0423ac388ed96beccb4276dd7c51d4d1269ab5 - I removed them as they are no longer called anywhere.

drojf commented 2 years ago

I have just applied a fix (https://github.com/07th-mod/higurashi-assembly/commit/0de0e97588a6f634d61ddefd21effe521600c99e) for all chapters (see https://github.com/07th-mod/higurashi-rei/issues/4).

The easiest way check whether the user has the updated DLL with the fix if they have a button on the Audio menu to open the BGM/SE FAQ on the wiki (earlier versions of the DLL do not have this button)

You can also check if on the game log they have a line like GameSystem: Starting GameSystem - DLL Version: 0.0.0.0

(the version will always be 0.0.0.0 for now due to me implementing versioning but not having any easy way to set it from our Travis CI builds)

If the fix I applied is ever encountered, the game will generate a mod_log_[N].txt file for each session, next to the output_log.log/Player.log, which states some debug info relating to when the fix triggered.