Resolves issues with loading 'Content' for PSM and OSX. #2223

Closed
wants to merge 4 commits into
from

Projects

None yet

5 participants

@DrMiaow
Contributor
DrMiaow commented Feb 10, 2014

This brings the behavior into alignment with XNA so that 'Content' is located
consistently relative to the ContentManager.RootDirectory.

@DrMiaow DrMiaow Resolves issues with loading 'Content' for PSM and OSX.
This brings the behavior into alignment with XNA so that 'Content' is located
consistently relative to the ContentManager.RootDirectory.
de5a801
@DrMiaow
Contributor
DrMiaow commented Feb 10, 2014

I've wrapped the absolute path in a #if OSX || PSM
I also attempt to reconstruct the path back to a local path if is under TitleContainer.Location

This works on PSM emulator and on a PS Vita.
I have tested this on OSX.

I have seen the same code/fix resolving issues in iOS and Android in the past but I have been unable to test that today.

I have a project that I regularly check with all the platforms and this change has made 'Content' loading behave more consistent across them all.

@DrMiaow DrMiaow commented on an outdated diff Feb 10, 2014
MonoGame.Framework/Content/ContentManager.cs
@@ -323,9 +354,13 @@ protected T ReadAsset<T>(string assetName, Action<IDisposable> recordDisposableO
catch (ContentLoadException ex)
{
//MonoGame try to load as a non-content file
-
+
+#if PSM || OSX
@DrMiaow
DrMiaow Feb 10, 2014 Contributor

This should be #if PSM || MONOMAC

@mgbot
Member
mgbot commented Feb 10, 2014

Can one of the admins verify this patch?

@mgbot
Member
mgbot commented Feb 10, 2014

Can one of the admins verify this patch?

@DrMiaow DrMiaow commented on an outdated diff Feb 10, 2014
MonoGame.Framework/TitleContainer.cs
@@ -109,7 +111,24 @@ static TitleContainer()
public static Stream OpenStream(string name)
{
// Normalize the file path.
- var safeName = GetFilename(name);
+ var safeName = GetFilename(name);
+
+#if OSX || PSM
@DrMiaow
DrMiaow Feb 10, 2014 Contributor

This should be if PSM || MONOMAC

@DrMiaow
Contributor
DrMiaow commented Feb 10, 2014

Just round tripped this patch back to OSX and fixed where I was using 'OSX' instead of 'MONOMAC'.

@KonajuGames
Contributor

I'm curious to know more about these issues you had. Aside from PSM, which
I know about, what were the issues you had with OS X, iOS and Android?
These platforms, especially iOS and Android, have been extensively tested
with no issues relating to requiring an absolute path.​

@DrMiaow
Contributor
DrMiaow commented Feb 11, 2014

My assumption is that Content should work the same way as XNA so loading content with a name should work cross platform.

XNA places a Content folder in the same folder as as the DLLs and executables, so this is what I have done with all my projects so that any code accessing the content relative to the execution path will work.

So when I build I have something like this for all projects

/bin/Release/
/bin/Release/Content/

The issue is that for OSX, when I initialise with.

Content.RootDirectory = "Content";

And then later on try to load some content.

_song = Content.Load<Song>("intro");

Then the song file fails to load because it is not found.

I need to use a combination of this on initialisation

Content.RootDirectory = AppDomain.CurrentDomain.BaseDirectory + "Content";

and my absolute path resolution to resolve this.

Is the issue that 'Content' needs to go into different locations for different platforms and I'm fighting against an undocumented convention?

@SamuelEnglard
Contributor

The thing is that on OSX the content is expected to be inside the app container currently.

@DrMiaow
Contributor
DrMiaow commented Feb 11, 2014

Is the 'App Container' the same location as the executable?

From memory it was looking in ../Resources/

I will check again tonight..

@SamuelEnglard
Contributor

Of sorts. Its a folder that ends in ".app" that on OSX is treated as the whole application. It should contain everything the program needs.

Off the top my head by default MonoGame will look in AppFolder.app/Content/Content for the content files. To get files there you add the to your project under a folder called Content and set the build action to Content

@DrMiaow
Contributor
DrMiaow commented Feb 11, 2014

OK. I'll take a look at this again tonight. So it sounds like we are looking at this for OSX

Program.app
Program.app/MonoBundle  <-- DLLs
Program.app/Content/Content  <-- Content

I'll check again tonight but I have my content set up the same way and it ends up here.

Program.app
Program.app/MonoBundle  <-- DLLs
Program.app/MonoBundle/Content  <-- Content
@SamuelEnglard
Contributor

Just got my hands on my Mac and checked the output. What the structure should look like for a MonoGame is

Program.app
Program.app/MonoBundle  <-- DLLs and the EXE
Program.app/Resources/Content   <-- Content
@DrMiaow
Contributor
DrMiaow commented Feb 11, 2014

Mine is different again

Program.app
Program.app/Contents/MonoBundle  <-- DLLs and the EXE
Program.app/Contents/MonoBundle/Content <-- Content
Program.app/Contents/Resources <-- monogameicon.png

My Content is added via a dedicated content project that is references by the main app.

@DrMiaow DrMiaow Removed changes to MONOMAC - Referenced Content project issue was not…
… issue.

Issue is that Referenced projects containing Content deploy that content relative to the DLL EXE.
I have given up and am just adding content to the main project for now.
In future it may be nice to failover content load to DLL EXE relative location.
4b4a222
@DrMiaow
Contributor
DrMiaow commented Feb 11, 2014

I have changed from having a separate shared project for my content and am now embedding content directly into the project.

I suspect this is my problem with the other platforms.

The good news is that now I can have

Content.RootDirectory = "Content";

instead of

Content.RootDirectory = AppDomain.CurrentDomain.BaseDirectory + "Content";

in my App.

On the negative side I had separate projects before including their own content quite elegantly, now, not so much. :(

Now that I know a bit better as to how the Content mapping works, I'll double check the PSM solution and see if I can make it more elegant.

@DrMiaow
Contributor
DrMiaow commented Feb 12, 2014

I can't see a way to make the PSM solution more elegant at this point.

  • The default working directory is "/"
  • All the content is deployed relative to "/Application"
  • You can't change the working directory (Security Exception).
  • There is no constant anywhere that defines "/Application" so a hard-coded string it stays.

So in my code with the PSM fix I can set Content.RootDirectory = "Content"; on init, just like I do for the other platforms and loading Content works.

Where I am doing my own file access to Content I have lots of #ifdef PSM where I need to prefix "/Application/Content/myfile.xml" to my paths.

If I use TitleContainer.OpenStream("Content/myfile.xml") then this goes away.

_So I'm proposing that this patch please be accepted for PSM and I will submit subsequent patches that do the following._

1) Convenience Functions in TitleContainer and ContentManager

Exposes methods in TitleContainer in the same vein as OpenStream for determining filepaths relative to the Application and
expose methods in ContentManager to provide access to streams and filepaths relative to ContentManager.RootDirectory for accessing files relative to the Content.

This will mean that code for different platforms will no longer need to duplicate the logic built into TitleContainer and ContentManager for working out the true path to files.

Eg. This would give me the choice of TitleContainer.OpenStream("Content/myfile.xml") or ContentManager.OpenStream("myfile.xml").

It would certainly make my code cleaner as well as providing support for the current 'convention' of how content works in MonoGame.

2) Allow 'Project Content' to Work Transparently.

After that I think maybe another patch that gets ContentManager to, as a final exception failover to attempt to locate the content relative to the .EXE/.DLL which would allow projects that contain their own content to be used without resorting to trickery with ContentManager.RootDirectory. EIther that or at least some instruction in the code that explains how the interaction between TitleContainer and ContentManager works and how to get 'Project Content' to work as opposed to 'Application Content'.

@DrMiaow
Contributor
DrMiaow commented Feb 14, 2014

Do I have to do anything else?

@DrMiaow
Contributor
DrMiaow commented Feb 21, 2014

What is the next steps on this please? Should I resubmit it? I can't see it as being merged?

@DrMiaow
Contributor
DrMiaow commented Feb 28, 2014

bump

@SamuelEnglard
Contributor

I don't have the power to make that decision but @tomspilman does

@tomspilman
Member

So i really don't understand why this particular issue has been so difficult. @DrMiaow please educate me here.

This PR seems to suggest that...

 TitleContainer.OpenStream("Content/myfile.xml");

... doesn't currently work. Is that correct? Why is that?

@tomspilman
Member

Bump!

@tomspilman
Member

Gonna give this a few more days to get a response before closing it.

@DrMiaow
Contributor
DrMiaow commented Apr 28, 2014

Sorry - been indisposed :)

Yes. In short PSM target as it stands seems to get the root directory wrong. My patch seems to resolve this.

I'm proposing in #2223 (comment) that I follow this up with some convenience functions to enable game code to be platform agnostic by explicitly implementing the 'convention' of where content should default to.

@tomspilman
Member

In short PSM target as it stands seems to
get the root directory wrong.

How is it wrong? Can you give me an example which covers:

  • The true absolute path to the file you are wanting to open.
  • The path that is passed into TitleContainer.OpenStream.
  • What the incorrect absolute path TitleContainer.OpenStream is generating.
@tomspilman
Member

I hate doing this, but i'm gonna close this PR.

I know it looks like I hate PSM... I don't. I just don't like seeing a bug fixed by what seems clearly to me to be the wrong changes.

This is like the 3rd PR for this issue and as of yet no one has been able to show me why this change needs to be made. If you or anyone else can answer my questions above we can always reopen this PR.

@tomspilman tomspilman closed this Apr 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment