Skip to content
This repository has been archived by the owner on Nov 19, 2018. It is now read-only.

[WIP] TrueCraft FNA #228

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

[WIP] TrueCraft FNA #228

wants to merge 30 commits into from

Conversation

flibitijibibo
Copy link
Contributor

This is less of a pull request and more of an experimental branch of TrueCraft that uses FNA over MonoGame for the TrueCraft.Client library. The game should function exactly the same as it did before.

All of the changes here are strictly changes to make the game XNA4-compliant. This includes API changes such as the SpriteBatch.Begin calls and runtime changes such as setting the GraphicsProfile to HiDef, as XNA4 would need for the 32-bit index buffers.

Here's a screenshot of this branch in action:

screenshot from 2016-02-02 13-03-15

This branch's performance has not yet been compared to the original master branch.

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

Earlier PR for comparison/discussion: #148

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

Initial thoughts:

  • This references FNA as a submodule, but you haven't added the submodule
    • The last time around I was concerned about making people build FNA in the first place, NuGet packages are still not published?
  • On Linux I had some issues with water textures loading with incorrect colors without using PngReader. Is that an issue here?

What is in your TODO list? This PR has [WIP] on the title, wondering what all remains to be done. I noticed, for example, that your screenshot has no hotbar.

Can you also summarize the reasons FNA is worth it over MonoGame?

@flibitijibibo
Copy link
Contributor Author

For now this is just a sample case for FNA's accuracy and not meant to be used in production for any real reason. There are likely performance benefits with our renderer, but this hasn't been measured yet.

I've not made NuGet packages only because there isn't a way to package as source and not as binary, and I don't want to publish FNA as binaries even for NuGet.

I'm having a look at the textures with apitrace now.

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

For now this is just a sample case for FNA's accuracy and not meant to be used in production for any real reason. There are likely performance benefits with our renderer, but this hasn't been measured yet.

So you're not looking to get it merged? How would you like me to treat this PR?

I've not made NuGet packages only because there isn't a way to package as source and not as binary, and I don't want to publish FNA as binaries even for NuGet.

I'm curious about why you've made that choice. I notice you publish binaries for all of your dependencies, for example.

I'm having a look at the textures with apitrace now.

A simple visual inspection of water should do the trick 😉 if it's purple, it's borken.

@flibitijibibo
Copy link
Contributor Author

I publish the native bins, but all the managed assemblies are what people usually need to debug - XNA inaccuracies are hard to catch without the ability to step through FNA.

A good example caught quickly with breakpoints in FNA: it turns out some of the rendering code depends on MonoGame's GL inaccuracy with regard to things like depth bias (glPolygonOffset); a possible reason why things like the HUD aren't showing up is because of rasterizer/blend/depth states varying between MonoGame and FNA.

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

Well, I have more to say about that, but it's not really on topic for this PR. Feel free to reach out externally if you want to hear my thoughts on that.

In my role as the TrueCraft maintainer, though, how do you want me to deal with this PR? Do you want it considered for merge? I'm still not sure what you want me to do with this PR.

@flibitijibibo
Copy link
Contributor Author

For now, just leave it as-is and don't worry about merging it. This will only ever matter if it's possible to have both MonoGame and FNA rendering correctly with the same code, but at the same time other people may be interested in this if MonoGame does not work correctly for whatever reason (MG dependencies like SDL 1.2 and OpenTK, etc.).

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

Alright. Do you want my feedback on your changes?

@flibitijibibo
Copy link
Contributor Author

Give me a few days to poke around the GL a little bit more and I'll put together a build for people to try out.

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

Alright, good luck!

@flibitijibibo
Copy link
Contributor Author

Quick update on the HUD, might affect the MonoGame build depending on OpenTK's behavior per-target: ScaleFactor is not set unless the client size is changed after the window is made. FNA/XNA only call that function when the user resizes a resizable window, so the scale was never getting set. This fixes the HUD for FNA:

https://github.com/flibitijibibo/TrueCraft/commit/3eafa3a94a8a8e846da834a92c71f0cfd5feb939

That said, if TC ever gets a resolution option in-game, that callback may need to be something like GraphicsDevice.DeviceReset instead.

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

Nice find. For what it's worth, I doubt TrueCraft will have in-game options in the foreseeable future.

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

As an aside, I'd also be curious to see how you'd deploy TrueCraft with FNA (with the Linux installer and the OSX archive) to deal with the launcher.

@flibitijibibo
Copy link
Contributor Author

I'll be honest, the Xwt dependency tree kind of scares me... I'm looking at the csproj files now and it somehow manages to have enough gunk in it to the point where it may as well just be System.Windows.Forms! It's definitely a tough call, but the good news is that the rest would be pretty easy to work with. Instead of calling mono you'd just call, for example, TrueCraft.Client.bin.x86_64. The bin suffix could be detected at runtime without much fuss, and you might even be able to optimize the binary size by reverting the recent change of statically linking the Mono runtime into the binary and have all the kick.bin.* files refer to that one libmonosgen lib.

But, packaging aside, I think I have the game rendering accurately now... modified this one depth bias value to be accurate to D3D:

https://github.com/flibitijibibo/TrueCraft/commit/8e7164977452d016500366b0b422c7893187765d

I also figured out why the water was purple for you - it actually had to do with both MonoGame's RenderTargetUsage accuracy as well as the final target draw's blending mode:

https://github.com/flibitijibibo/TrueCraft/commit/0e66c1421c302f221ce122db9d248499f15b0bea

The purple tint you were seeing was actually the recently-cleared backbuffer, which by default clears to that awful purple color by default in XNA. Of course, you draw to the entire screen and don't care about the previous contents anyway, so you can skip it with BlendState.Opaque. Even better, you don't even need to clear at all - on top of the previous contents of the backbuffer not being important, you also don't care about the RenderTarget's contents, since you clear to CornflowerBlue at the start of the frame. So you can actually skip a clear by making use of RenderTargetUsage.PreserveContents, giving us the behavior we want and making things a LOT faster than you'd expect!

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

I'll be honest, the Xwt dependency tree kind of scares me... I'm looking at the csproj files now and it somehow manages to have enough gunk in it to the point where it may as well just be System.Windows.Forms! It's definitely a tough call, but the good news is that the rest would be pretty easy to work with. Instead of calling mono you'd just call, for example, TrueCraft.Client.bin.x86_64. The bin suffix could be detected at runtime without much fuss, and you might even be able to optimize the binary size by reverting the recent change of statically linking the Mono runtime into the binary and have all the kick.bin.* files refer to that one libmonosgen lib.

Thanks for the insight. At some point it might make sense to move the launcher code into an in-game GUI, but that's quite an effort.

But, packaging aside, I think I have the game rendering accurately now... modified this one depth bias value to be accurate to D3D

Cool. This code was written pretty blindly to adjust the rendering of the outline around the block your mouse points to. Does that still work as expected?

I also figured out why the water was purple for you - it actually had to do with both MonoGame's RenderTargetUsage accuracy as well as the final target draw's blending mode

Nice, I'm glad this ends with better performance too.

@flibitijibibo
Copy link
Contributor Author

The DepthBias fix works for me; my test case was the cracking sprite when destroying a block. The change just takes in the calculation we normally do to recreate the D3D-like bias and inverts it, so the result is '-3' as the client was originally expecting.

The only GUI lib I've ever shipped without feeling like I've done something horribly offensive is ImGui, but it doesn't really have anything for web rendering like the launcher wants at the moment:

https://github.com/ocornut/imgui

It does work with GPU rendering though, as I did with file dialogs for TowerFall Ascension:

https://github.com/flibitijibibo/XNAFileDialog

@ddevault
Copy link
Owner

ddevault commented Feb 2, 2016

Oh man that interop is awful, I'd sooner just make my own GUI library with the Minecraft L+F

@flibitijibibo
Copy link
Contributor Author

Yeah... life with C++ instead of C! :(

@flibitijibibo
Copy link
Contributor Author

An FNA build is downloadable from here:

http://www.flibitijibibo.com/TCFNA.tar.bz2

This only provides the native libs used by FNA and modifies the script to add the folders to the LD_LIBRARY_PATH, it does not provide the full MonoKickstart runtime due to the Xwt dependency. But, as long as you have Mono 4.x and Xwt's dependencies, this package should work just fine.

@flibitijibibo
Copy link
Contributor Author

I did a bit of an ugly thing to Paths.Base just now - it sidesteps SpecialFolder to get working paths on OSX when the time comes for that target. The ugly part is the platform detection... Normally I just use SDL_GetPlatform() but without linking to FNA I had to improvise. It should be fine until someone tries running this on one of the BSDs or something, in which case we just have to use shell instead of checking for the OSX /home/ folder. I'll likely change this before that happens either way, but just FYI!

@ddevault
Copy link
Owner

ddevault commented Feb 3, 2016

@flibitijibibo
Copy link
Contributor Author

Oh ho, glad to see I don't have to write that then! Will update tomorrow morning.

@flibitijibibo
Copy link
Contributor Author

This branch has been updated for the latest changes. Note that this may still crash due to problems with Xwt.Gtk3, as I had this crash too:

[ERROR] FATAL UNHANDLED EXCEPTION: System.Reflection.ReflectionTypeLoadException: The classes in the module cannot be loaded.
  at (wrapper managed-to-native) System.Reflection.Assembly:GetTypes (System.Reflection.Assembly,bool)
  at System.Reflection.Assembly.GetTypes () [0x00000] in /builddir/build/BUILD/mono-4.0.5/mcs/class/corlib/System.Reflection/Assembly.cs:360 
  at TrueCraft.Core.Logic.BlockRepository.DiscoverBlockProviders () [0x00021] in /home/flibitijibibo/Programming/csProjects/TrueCraft/TrueCraft.Core/Logic/BlockRepository.cs:30 
  at TrueCraft.Launcher.Singleplayer.Worlds.Load () [0x0002c] Vector smash protection is enabled.
in /home/flibitijibibo/Programming/csProjects/TrueCraft/TrueCraft.Launcher/Singleplayer/Worlds.cs:24 
  at TrueCraft.Launcher.Views.SingleplayerView..ctor (TrueCraft.Launcher.LauncherWindow window) [0x00016] in /home/flibitijibibo/Programming/csProjects/TrueCraft/TrueCraft.Launcher/Views/SingleplayerView.cs:35 
  at (wrapper remoting-invoke-with-check) TrueCraft.Launcher.Views.SingleplayerView:.ctor (TrueCraft.Launcher.LauncherWindow)
  at TrueCraft.Launcher.LauncherWindow..ctor () [0x00087] in /home/flibitijibibo/Programming/csProjects/TrueCraft/TrueCraft.Launcher/LauncherWindow.cs:40 
  at (wrapper remoting-invoke-with-check) TrueCraft.Launcher.LauncherWindow:.ctor ()
  at TrueCraft.Launcher.Program.Main (System.String[] args) [0x00097] in /home/flibitijibibo/Programming/csProjects/TrueCraft/TrueCraft.Launcher/Program.cs:36

The fix is to go to Program.cs:Main() and change the Gtk3 line to just Gtk.

@flibitijibibo
Copy link
Contributor Author

Digging this old thing up from the grave... the branch still doesn't do anything remarkable and appears to work as much as it doesn't appear to be explicitly breaking anything, but the engine doesn't appear to want to render any blocks (as in, no GL calls happen for FNA to send out). I did notice that initial world loading had this inside...

"Threadpool worker"  at System.Collections.Generic.Stack`1<TrueCraft.Profiling.Profiler/ActiveTimer>.Pop () <0x0007d>
  at TrueCraft.Profiling.Profiler.Done (long) <0x000ab>
  at TrueCraft.Core.Lighting.WorldLighting.LightVoxel (int,int,int,TrueCraft.Core.Lighting.WorldLighting/LightingOperation) <0x00d83>
  at TrueCraft.Core.Lighting.WorldLighting.LightBox (TrueCraft.Core.Lighting.WorldLighting/LightingOperation) <0x0055b>
  at TrueCraft.Core.Lighting.WorldLighting.TryLightNext () <0x00127>
  at TrueCraft.Core.Lighting.WorldLighting.InitialLighting (TrueCraft.API.World.IChunk,bool) <0x0028b>
  at TrueCraft.MultiplayerServer.HandleChunkGenerated (object,TrueCraft.API.World.ChunkLoadedEventArgs) <0x00157>
  at (wrapper delegate-invoke) System.EventHandler`1.invoke_void_object_TEventArgs (object,TEventArgs) <0x000b2>
  at (wrapper delegate-invoke) System.EventHandler`1.invoke_void_object_TEventArgs (object,TEventArgs) <0x000b2>
  at (wrapper delegate-invoke) System.EventHandler`1.invoke_void_object_TEventArgs (object,TEventArgs) <0x000b2>

// Like a jillion more of these

  at (wrapper delegate-invoke) System.EventHandler`1.invoke_void_object_TEventArgs (object,TEventArgs) <0x000b2>
  at (wrapper delegate-invoke) System.EventHandler`1.invoke_void_object_TEventArgs (object,TEventArgs) <0xffffffff>
  at TrueCraft.Core.World.World.OnChunkGenerated (TrueCraft.API.World.ChunkLoadedEventArgs) <0x0002f>
  at TrueCraft.Core.World.Region.GenerateChunk (TrueCraft.API.Coordinates2D) <0x0017f>
  at TrueCraft.Core.World.Region.GetChunk (TrueCraft.API.Coordinates2D,bool) <0x000a7>
  at TrueCraft.Core.World.World.GetChunk (TrueCraft.API.Coordinates2D,bool) <0x00107>
  at TrueCraft.RemoteClient/<UpdateChunks>c__AnonStorey1.<>m__0 () <0x000b8>
  at System.Threading.Tasks.Task.InnerInvoke () <0x00051>
  at System.Threading.Tasks.Task.Execute () <0x0004c>
  at System.Threading.Tasks.Task.ExecutionContextCallback (object) <0x00053>
  at System.Threading.ExecutionContext.Run (System.Threading.ExecutionContext,System.Threading.ContextCallback,object) <0x00060>
  at System.Threading.ExecutionContext.Run (System.Threading.ExecutionContext,System.Threading.ContextCallback,object,bool) <0x00017>
  at System.Threading.Tasks.Task.ExecuteWithThreadLocal (System.Threading.Tasks.Task&) <0x00137>
  at System.Threading.Tasks.Task.ExecuteEntry (bool) <0x000bf>
  at System.Threading.Tasks.Task.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem () <0x0000f>
  at System.Threading.ThreadPool.<UnsafeQueueCustomWorkItem>m__0 (object) <0x000ae>
  at (wrapper runtime-invoke) <Module>.runtime_invoke_void__this___object (object,intptr,intptr,intptr) <0xffffffff>

Not sure what that's about, but maybe it's getting stuck somewhere before block buffers can be generated. In any case, now that TC seems to be picking up again I thought I'd at least get the FNA version updated.

@ddevault
Copy link
Owner

ddevault commented Jun 8, 2017

A rewrite to OpenTK straight-up is in the pipeline.

@flibitijibibo
Copy link
Contributor Author

Whatever works. I may do a similar branch that replaces the non-GL/AL bits with SDL2# and wedges MiniTK into the middle for those bindings. Same idea, different libs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants