ValveSoftware / Proton

Compatibility tool for Steam Play based on Wine and additional components
Other
24.44k stars 1.07k forks source link

Let's Build a Zoo (1547890) #5288

Open screwylightbulb opened 3 years ago

screwylightbulb commented 3 years ago

Compatibility Report

System Information

I confirm:

Symptoms

RESOLVED: Game crashes just after opening logo animation NEW: The above issue has a workaround to just skip the music when it nullrefs, (assuming it's a simple try...catch) but the root problem still exists. Currently the observable symptom is that there is no music when running with Proton.

Reproduction

Just start the game

steam-1547890.log

Relevant bit seems to be: ``` [ERROR] FATAL UNHANDLED EXCEPTION: System.NullReferenceException: Object reference not set to an instance of an object at SEngine.Content.MonogameEmbedLoad.LoadMP3SongFromEmbeddedResource (System.String FilePathFromContentFolder) [0x00075] in :0 at SEngine.Content.LoadSongForPlatform.LoadThisSong (Microsoft.Xna.Framework.Content.ContentManager contentManager, System.String FilePathFromContentFolder) [0x00000] in :0 at TinyZoo.Audio.MusicManager.LoadThisSong (TinyZoo.Audio.SongTitle songtitle) [0x000a3] in <8ec0f307dfe0413b826235f770724f40>:0 at TinyZoo.Audio.MusicManager.LoadMusic () [0x00042] in <8ec0f307dfe0413b826235f770724f40>:0 at TinyZoo.AssetContainer.LoadAssetsDuringSplash (Microsoft.Xna.Framework.Content.ContentManager contentmanager, SpringIAP.SpringIAPManager springIAPmanager, TinyZoo.Player player, System.Int32& Index, System.Boolean& IsDone) [0x00116] in <8ec0f307dfe0413b826235f770724f40>:0 at TinyZoo.SplashScreen.SplashManager.UpdateSplashManager (TinyZoo.Player[] players, Microsoft.Xna.Framework.Content.ContentManager contentmanager, System.Single DeltaTime) [0x0013c] in <8ec0f307dfe0413b826235f770724f40>:0 at TinyZoo.GameStateManager.UpdateGameStateManager (System.Single DeltaTime, TinyZoo.Player[]& players, Microsoft.Xna.Framework.Content.ContentManager Content, SpringIAP.SpringIAPManager springIAPmanager, Microsoft.Xna.Framework.GraphicsDeviceManager graphics, Microsoft.Xna.Framework.Graphics.GraphicsDevice _GraphicsDevice) [0x00172] in <8ec0f307dfe0413b826235f770724f40>:0 at TinyZoo.Game1.Update (Microsoft.Xna.Framework.GameTime gameTime) [0x001c6] in <8ec0f307dfe0413b826235f770724f40>:0 at Microsoft.Xna.Framework.Game.DoUpdate (Microsoft.Xna.Framework.GameTime gameTime) [0x00019] in <115d97b957eb41cabade1b29d1e7edf4>:0 at Microsoft.Xna.Framework.Game.Tick () [0x00127] in <115d97b957eb41cabade1b29d1e7edf4>:0 at MonoGame.Framework.WinFormsGameWindow.TickOnIdle (System.Object sender, System.EventArgs e) [0x00014] in <115d97b957eb41cabade1b29d1e7edf4>:0 at System.Windows.Forms.Application+ThreadContext.System.Windows.Forms.UnsafeNativeMethods.IMsoComponent.FDoIdle (System.Int32 grfidlef) [0x0001a] in <2736a4f873604bfea9e5f3a0d64b37d6>:0 at System.Windows.Forms.Application+ComponentManager.System.Windows.Forms.UnsafeNativeMethods.IMsoComponentManager.FPushMessageLoop (System.IntPtr dwComponentID, System.Int32 reason, System.Int32 pvLoopData) [0x00243] in <2736a4f873604bfea9e5f3a0d64b37d6>:0 at System.Windows.Forms.Application+ThreadContext.RunMessageLoopInner (System.Int32 reason, System.Windows.Forms.ApplicationContext context) [0x00282] in <2736a4f873604bfea9e5f3a0d64b37d6>:0 at System.Windows.Forms.Application+ThreadContext.RunMessageLoop (System.Int32 reason, System.Windows.Forms.ApplicationContext context) [0x0001a] in <2736a4f873604bfea9e5f3a0d64b37d6>:0 at (wrapper remoting-invoke-with-check) System.Windows.Forms.Application+ThreadContext.RunMessageLoop(int,System.Windows.Forms.ApplicationContext) at System.Windows.Forms.Application.Run (System.Windows.Forms.Form mainForm) [0x0000d] in <2736a4f873604bfea9e5f3a0d64b37d6>:0 at MonoGame.Framework.WinFormsGameWindow.RunLoop () [0x00011] in <115d97b957eb41cabade1b29d1e7edf4>:0 at MonoGame.Framework.WinFormsGamePlatform.RunLoop () [0x00000] in <115d97b957eb41cabade1b29d1e7edf4>:0 at Microsoft.Xna.Framework.Game.Run (Microsoft.Xna.Framework.GameRunBehavior runBehavior) [0x0008b] in <115d97b957eb41cabade1b29d1e7edf4>:0 at Microsoft.Xna.Framework.Game.Run () [0x0000c] in <115d97b957eb41cabade1b29d1e7edf4>:0 at TinyZoo.Program.Main (System.String[] args) [0x00015] in <8ec0f307dfe0413b826235f770724f40>:0 (1) ```
screwylightbulb commented 3 years ago

Small update from the developer.

we have temporarily tried going past this issue without letting it crash - there will just be no music in proton...or....it will crash somewhere else! thanks for the help - we will roll out this update....eventually....

Basically, a null reference error happens during the splash screen while loading an MP3 file. So it's either specific to that, or a general issue with loading from the Content assembly.

While I am sure this can be fixed from their side and their attention to the issue is much appreciated, I have to also wonder if there's not a fix that could happen from the Proton side that might benefit other projects.

Do XNA games use wine-mono?

screwylightbulb commented 3 years ago

Another update from the developer

They have pushed the above mentioned workaround. For now, on Proton, there is no music.

screwylightbulb commented 3 years ago

A comment from the developer on this:

We embed the resources, so proton doesn't understand the way the file handling is done - it uncompresses a file, loads it into memory - then removes it. Its not a very standard way of doing things, I am super happy that the temporary fix we put in let the game work. Hopefully someone amazing will figure out how to make things work, so Proton can load the music. We wouldn't need to change the game on our end, once proton is updated, the music will just load and play as normal.

FWIW, the source code of the game is very easy to browse using ILSpy, so this might help with debugging.

madewokherd commented 2 years ago

Hi, this is technically not an XNA game as it ships and uses MonoGame (same/similar API, different implementation). It is using wine-mono so it's very possible that's related. But just from the log I'm not able to tell what causes the null reference.

madewokherd commented 2 years ago

ILSpy shows that it's using reflection to try to access a private field of the IsolatedStorageFile class named m_RootDir, which doesn't exist in Mono's implementation of IsolatedStorageFile.

This is in SEngine.dll, so I'm not sure if it's the developer's code.

madewokherd commented 2 years ago

Mono's implementation uses a private DirectoryInfo field named directory for this.

madewokherd commented 2 years ago

It's possible to work around that error by adding a m_RootDir field that mirrors directory, but after that I run into a file sharing conflict trying to read the mp3.

madewokherd commented 2 years ago

Whoops, the conflict is actually in trying to delete the mp3. I think it's because our MFCreateFile function doesn't use the FILE_SHARE_DELETE flag when opening the file.

madewokherd commented 2 years ago

Those two changes are enough to get music working. I used this patch to Wine:

diff --git a/dlls/mfplat/main.c b/dlls/mfplat/main.c
index ff7155b5ade..58bfccb7264 100644
--- a/dlls/mfplat/main.c
+++ b/dlls/mfplat/main.c
@@ -4422,7 +4422,7 @@ HRESULT WINAPI MFCreateFile(MF_FILE_ACCESSMODE accessmode, MF_FILE_OPENMODE open
 {
     DWORD capabilities = MFBYTESTREAM_IS_SEEKABLE | MFBYTESTREAM_DOES_NOT_USE_NETWORK;
     DWORD filecreation_disposition = 0, fileaccessmode = 0, fileattributes = 0;
-    DWORD filesharemode = FILE_SHARE_READ;
+    DWORD filesharemode = FILE_SHARE_READ | FILE_SHARE_DELETE;
     struct bytestream *object;
     FILETIME writetime;
     HANDLE file;

And this patch to the mono submodule of Wine Mono:

diff --git a/mcs/class/corlib/System.IO.IsolatedStorage/IsolatedStorageFile.cs b/mcs/class/corlib/System.IO.IsolatedStorage/IsolatedStorageFile.cs
index 55fba10cac9..91c553e6140 100644
--- a/mcs/class/corlib/System.IO.IsolatedStorage/IsolatedStorageFile.cs
+++ b/mcs/class/corlib/System.IO.IsolatedStorage/IsolatedStorageFile.cs
@@ -379,7 +379,20 @@ internal static ulong GetDirectorySize (DirectoryInfo di)

        // non-static stuff

-       private DirectoryInfo directory;
+       private DirectoryInfo _directory;
+       private string m_RootDir;
+       private DirectoryInfo directory
+       {
+           get
+           {
+               return _directory;
+           }
+           set
+           {
+               _directory = value;
+               m_RootDir = _directory.FullName + "/";
+           }
+       }

        private IsolatedStorageFile (IsolatedStorageScope scope)
        {

This needs tests still, I'm not sure if that MFCreateFile behavior is correct.

screwylightbulb commented 2 years ago

Hi, when you say "needs tests", do you mean it must be compiled and tested, or do you mean you need to write unit tests for it?

madewokherd commented 2 years ago

I needed to write unit tests. I submitted the Wine part yesterday: https://www.winehq.org/pipermail/wine-devel/2021-November/200600.html

screwylightbulb commented 2 years ago

Ah! Well thanks a lot for looking into this. I'm sure the devs will appreciate it too since they're a small team.

madewokherd commented 2 years ago

OK, so that mfplat commit has landed in the bleeding-edge beta of Proton experimental. It should work if you combine that version of Proton with the wine-mono build from https://github.com/madewokherd/wine-mono/actions/runs/1468027439 (scroll down on that page to find the "make msi output" link for download, extract the msi from the zip, and follow the instructions at https://github.com/redmcg/wine-mono/wiki#install-later-version to install that msi into the game's prefix).

Native .NET Framework might also work.

It should also be possible for the game's developer to work around the need for updated Wine Mono by putting their temporary .mp3 files in another location without relying on private fields (here's an example: https://stackoverflow.com/a/10152460/1248592). Then it will require the mfplat update but not the Wine Mono update. The mfplat update is needed to allow them to delete the mp3 file while playing it.

screwylightbulb commented 2 years ago

Replying to https://github.com/ValveSoftware/Proton/issues/5288#issuecomment-974359397

Woop woop! Gave it a quick test and it works 100%. Thanks. :) I'll wait for it to make its way through to a stable Proton and then update this report and close it.

In the mean time I will pass your suggestion on to the developer regarding just doing it differently. If the change is not going to really benefit (m)any other titles, that'd be better anyway.

screwylightbulb commented 2 years ago

Update. This latest fix does make the menu music work, but in-game still broken. I have not done any troubleshooting yet though, and I think it might be best to first talk to the developers, which I will arrange.

madewokherd commented 2 years ago

Welp, this is why I'm not in QA. Normally someone would actually test things like starting a game, but since it was going to take a while for the wine-mono changes to make their way into a release, I wanted to get that out sooner.

madewokherd commented 2 years ago

It seems the menu music only works the first time. If you go back from the game to the menu, there's no music.

There are some mfplat messages that might be relevant.

3356872.980:010c:01a0:fixme:mfplat:topology_loader_Load 20375D30, 2032F970, 18C9FD74, 00000000.
3356872.992:010c:01a0:fixme:mfplat:audio_renderer_get_service_GetService Unsupported service {866fa297-b802-4bf8-9dc9-5e3b6a9f53c9}, interface {0a9ccdbc-d797-4563-9667-94ec5d79292d}.
3356873.005:010c:01a0:fixme:mfplat:audio_converter_ProcessMessage Unhandled message type 10000003.
3356873.005:010c:01a0:fixme:mfplat:media_source_QueryInterface {6ef2a662-47c0-4666-b13d-cbb717f2fa2c}, 18C9FD1C.

That service GUID is MF_RATE_CONTROL_SERVICE. The missing interface is IMFClockConsumer. I don't really know mfplat so I can't say which of these are likely to be related.

screwylightbulb commented 2 years ago

We tried the 7.0-4 release candidate which says it fixes the music, however it still seems to be the same thing where the title music works, but not in-game.

alasky17 commented 2 years ago

@screwylightbulb Thank you for the report - unfortunately that is the expected behavior right now. Some music playback in the game was improved, but the in-game music is still unsupported. I've removed the game from the changelog to avoid further confusion :)