Memoria icon indicating copy to clipboard operation
Memoria copied to clipboard

Crash when Vivi use Fire on Garnet (Theatre scene)

Open DV666 opened this issue 2 years ago • 17 comments

Hi !

I was on the last stretch to post the demo of my mod... except I got a bad surprise when i test the beginning :(

Indeed, when Vivi cash Fire on Garnet's head, the game crash with the last Memoria build (with SFX Rework) : so we are dealing with a major bug!

image

I thought it was the fire animation that caused the crash... but nope. The culprit is the sound !

After some tests with Hades Workshop, when i remove the RunSoundCode3 from Vivi script, no more crash.

Vivi_Loop in Field 67 (Prima Vista)

case 4: set VAR_GlobUInt8_26 = 4 Wait( 1 ) RunSoundCode3( 53248, 619, 0, -128, 125 ) RunSharedScript( 6 ) RunAnimation( 12598 ) WaitAnimation( ) RunAnimation( 12596 ) Wait( 10 ) set VAR_GlobUInt8_25 = 1 > if ( 0 ) { RunSoundCode3( 53248, 1545, 0, -128, 125 ) <=== THIS } WaitAnimation( ) RunAnimation( 12594 ) set VAR_GlobUInt8_26-- while ( VAR_GlobUInt8_26 > 0 ) { set VAR_GlobBool_230 = 1 Wait( 1 ) } while ( VAR_GlobBool_230 == 1 ) { Wait( 1 ) } set VAR_GlobBool_231 = 1 break

So, there's something in the assembly-csharp.dll with the last Memoria build that makes this function messed up ^^.

Edit: Or maybe the game just can't find the sound with the ID 1545 it seems... I'm trying to see on my side!

DV666 avatar Jan 26 '22 19:01 DV666

After some further investigation, I confirm that it is this specific sound that crashes the game. When i change the ID, it's works perfectly.

So i found something that on the assembly-charp (this code was already present on Memoria 15 October Build and no issue with it)

FF9Snd.cs

private static void ParameterChanger(ref Int32 ParmType, ref Int32 ObjNo, ref Int32 Arg1, ref Int32 Arg2, ref Int32 Arg3) { if ((ParmType >> 12 & 3) == 0) { if (ObjNo == 93) { ObjNo = 5; } else if (ObjNo == 137) { ObjNo = 10; } else if (ObjNo == 78) { ObjNo = 22; } } if ((ParmType >> 12 & 3) == 1) { if (ObjNo == 3067 || ObjNo == 3068) { ObjNo = 1748; } else if (ObjNo == 293) { ObjNo = 633; } else if (ObjNo == 1545) { ObjNo = 58; } else if (ObjNo == 679) { ObjNo = 9010197; } else if (ObjNo == 1230 || ObjNo == 1231) { FF9Snd.FF9AllSoundDispatch(ParmType, 9000023, Arg1, Arg2, Arg3); FF9Snd.FF9AllSoundDispatch(ParmType, 9000024, Arg1, Arg2, Arg3); FF9Snd.FF9AllSoundDispatch(ParmType, 9000025, Arg1, Arg2, Arg3); } else if (ObjNo == 681) { ObjNo = 3092; } else if (ObjNo == 672) { ObjNo = 9010138; } else if (ObjNo == 678) { ObjNo = 725; } else if (ObjNo == 3084) { ObjNo = 370; } } }

When i put theses lines on comments, no more crash :

  	else if (ObjNo == 1545)
  	{
  		ObjNo = 58;
  	}

However, I don't understand why this part make the game crashes ^^'

DV666 avatar Jan 26 '22 20:01 DV666

Yesterday, it seemed to me that this bug was present on a "vanilla" instance with the last build of Memoria (SFX Rework) but today, I can't reproduce it anymore...

Maybe it's from my side, because my .dll has been modified quite a bit but I don't understand why... weird.

I think you should just test this passage on your side and if it's OK: you can close the ticket ^^ (and sorry for the disturbance if that's the case).

DV666 avatar Jan 27 '22 09:01 DV666

I can't do anything for now regarding Memoria nor FF9 but I did a couple of full runs with the v2021.11.17 before and it never crashed on me at that point. Maybe it still some problem with the ambient sound fix from a couple of releases ago (that's the closest part recently modified concerning "RunSoundCode3"... but why it crashes, that would be a mystery) but then it wouldn't be a consistent crash.

Tirlititi avatar Jan 27 '22 17:01 Tirlititi

That's what I thought: it seems that comes from my side.

However, I don't understand why it crashes with my .dll: I can't see what I changed that would affect the sound ah ah weird! But well, I found a fix in the meantime ^^

I close the ticket... Sorry for the inconvenience ! :(

DV666 avatar Jan 27 '22 18:01 DV666

Are you sure about:

RunSoundCode3( 53248, 1545, 0, -128, 125 ) <=== THIS

Perhaps Tirlity and I interpret sounds differently, but it confuses me that you have a negative parameter:

            // se050118
            @aud.SfxPlay("se050118", volume: 125); // FLDSND(op: 53248, sound: 2973, a1: 0, a2: 128, a3: 125)

Albeoris avatar Jan 27 '22 20:01 Albeoris

The line "RunSoundCode3" in Hades Workshop has different effects depending on the operation used ("53248" here). The arguments you call a1, a2 and a3 can be of different types depending on that operation, sometimes signed sometimes not. By default in Hades Workshop, it is displayed as a signed integer on 8 bits even though it should surely be interpreted as an unsigned integer on 8 bits on that particular instance. In terms of binary values, a signed "-128" is the same as an unsigned "128".

"Whether a number should be interpreted as signed or unsigned" is not a decision that Hades Workshop takes flawlessly. But then again, it has no impact on the code at a binary level.

Tirlititi avatar Jan 27 '22 21:01 Tirlititi

I'm sorry to have dug up this topic but I have new information about this bug (thanks to Invicta on Steam for giving the clue).

Indeed, I managed to reproduce it on the latest version of Memoria (as well as the most recent build): this bug occurs when you activate the sounds import function in the Memoria.ini !

So... I don't know why it's doing this xD It has something to do with these lines because when you delete them, it no longer crashes the game:

FF9Snd.cs https://github.com/Albeoris/Memoria/blob/b85dd4cd7b628e0cd13548b659a67bf754ff6676/Assembly-CSharp/Global/ff9/Snd/FF9Snd.cs#L228-L231

But I'd like to have your opinion to know why it makes the game crash like this ^^

EDIT : According to the person who reported this bug, it also does so when Vivi uses his fire in the Ice Cavern too.

DV666 avatar Mar 26 '22 17:03 DV666

Ok, I think the bug comes from the fact the "ParameterChanger" applies halfway through the process of playing the sound...

The flow of execution should be something like this:

  1. In the related field scripts, there is a line RunSoundCode3( 53248, 1545, 0, 128, 125 ) (explanation: "RunSoundCode3" is the operation called FLDSND3 in the source code and "53248" is the sound code for playing a sound).
  2. I think that this line is a call to the function FF9FieldSoundDispatch when used on the field. In that function, there is some hashset keeping track of sounds that have been played surely for optimization purpose. At that point, the sound ID has NOT been changed yet and is still 1545.
  3. FF9FieldSoundDispatch then calls the more generic FF9AllSoundDispatch that starts by swapping the sound ID from 1545 to 58 (with a call of that ParameterChanger method you spotted) and then proceed to play the sound.

So I think that a proper fix is to make sure the ParameterChanger is done before doing anything else, so the sound ID is consistent through the whole process. It should be OK to add a line FF9Snd.ParameterChanger(ref ParmType, ref ObjNo, ref Arg1, ref Arg2, ref Arg3); at the start of the method FF9FieldSoundDispatch (right after ParmType &= 65535;) and possibly do that as well in the method FF9WorldSoundDispatch to avoid the same potential problem for sounds played on the World Map.

That is, if we want to keep the sound 58 be used in place of the sound 1545 (which is what the devs planned and they maybe had a good reason for that; maybe the sound 1545 doesn't exist or something like that).

Can you please try to see if that solves the problem? Adding that line in FF9FieldSoundDispatch while keeping the method ParameterChanger untouched?

Tirlititi avatar Mar 26 '22 20:03 Tirlititi

I'm going to test that right now!

DV666 avatar Mar 26 '22 20:03 DV666

Ok so... i think i find the issue ^^

First of all, I tried your manipulation Tirlititi and... it still crashes unfortunately :( and I think I know what part of the code that makes the game crash !

It's seems to be this part :

https://github.com/Albeoris/Memoria/blob/b85dd4cd7b628e0cd13548b659a67bf754ff6676/Assembly-CSharp/Global/AllSoundDispatchPlayer.cs#L525-L545

When i remove the part with ObjNo == 58, no more crash... It's that a part from the core code ?

For information, the game crashes when it tries to read the file se02006.ogg (which corresponds to the ID 58) => By removing this part of the code, Memoria manages to read the new file through the import.

DV666 avatar Mar 26 '22 21:03 DV666

Ow, incredible... Could it be those SdSoundSystem_SoundCtrl_SetVolume calls that crash the game? If I understand these 2 lines correctly, they are meant to gradually increase the volume of the sound 58 from volume=0 to volume=0.7 in 300 milliseconds. Or are you saying that se02006.ogg is corrupted?

Tirlititi avatar Mar 26 '22 22:03 Tirlititi

Se02006.ogg is not corrupted because it's a sound used by Fire (single) => With my Sounds Fix or without, i have no issue with this ^^

When we remove this part of code on TuneUpSoundEffectByObjNo, Memoria can read perfectly the new sound Se02006.ogg which is imported.

DV666 avatar Mar 26 '22 22:03 DV666

So to be more precise about the problem if I wasn't clear before:

The sound that is played in the PSX version is the sound file Se050089.ogg (ID 1545).

However, the developers had the idea to do a trick like Choco's beak, which is to use another sound instead... (whereas Se050089.ogg does exist and the sound is quite correct).

They decided to use the se020006 (ID 58) sound file instead ( thus the lines in the FF9Snd), which is a file used by Fire (simple) and Fire Sword (of course).

So when you enable the audio import function with Memoria and it tries to play the new se020006 in this field... it crashes.

And I confirm that it is the TuneUpSoundEffectByObjNo part in AllSoundDispatchPlayer.cs that makes this crash because by removing it, Memoria manages to read the file (even if it is not the good one).

And finally, I understand why they prefer to use se020006 instead of se050089.... because the sound is really disgusting ! ahahah

So either we delete the problematic part in the TuneUpSoundEffectByObjNo or we have to play around with the parameters... when I'll have some time, I'll try to test that ^^

DV666 avatar Mar 27 '22 14:03 DV666

For informations, the se050089 from Steam Version : https://user-images.githubusercontent.com/17729817/160285416-e79ddb93-1433-43b8-93d3-221b50aee167.mp4

The one used by PSX Version : https://user-images.githubusercontent.com/17729817/160285457-c977a53f-bb30-416c-95b7-943547385fdc.mp4

DV666 avatar Mar 27 '22 14:03 DV666

@Tirlititi @DV666 was a fix pushed in a PR or is it not actually a problem for users?

snouz avatar Jan 14 '24 14:01 snouz

I think we wrote a hotfix for that crash by removing the problematic call to SdSoundSystem_SoundCtrl_SetVolume. We still don't know why exactly it caused that crash for some people (it wasn't consistent if I remember correctly).

Maybe a complete fix would require an update of the SdLib.dll.

Tirlititi avatar Jan 14 '24 23:01 Tirlititi

btw, I found an old bug report in moguri

image

snouz avatar Mar 01 '24 14:03 snouz