Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#4855 closed defect (fixed)

TOOLS: The media directory is not installed

Reported by: SF/c-korn Owned by: criezy
Priority: normal Component: Tools
Keywords: Cc:
Game:

Description

The Makefile.common only installs the $TARGETS but does not install the media directory to
$(DESTDIR)/($DATADIR)/scummvm-tools .

Maybe gui/main.cpp also needs to be fixed if wxStandardPaths::Get().GetResourcesDir() does not return the correct path. (line 530).

Ticket imported from: #2984217. Ticket imported from: bugs/4855.

Change History (12)

comment:1 by SF/c-korn, 10 years ago

This affects the current scumvm-tools version 1.1.0.

comment:2 by salty-horse, 10 years ago

According to the wx docs at <http://www.wxpython.org/docs/api/wx.StandardPaths-class.html#GetResourcesDir>, GetResourcesDir returns:
* Contents/Resources under Mac
* prefix/share/appname under Unix

The install script already assumes this for Mac bundling, but I think it should be configurable for Unix (by respecting $DATADIR).

Because of the same code, the media files don't load when running from the development dir (without installing), which should also be fixed.

comment:3 by salty-horse, 10 years ago

Component: Tools

comment:4 by criezy, 10 years ago

I have committed a fix in the trunk for several of the issues listed in the bug description and in salty-horse comment:
1) There is now a datadir option in configure (default to $prefix/share) and make install now copies the files from gui/media/ into $(DESTDIR)$(DATADIR)/scummvm-tools/
2) Changes the way the GUI loads the picture files so that it has more chances to succeed. In particular it should work without the need to install (i.e. when the files are in `pwd`/gui/media) or when wxStandardPaths::GetResourcesDir() is not correct (which is the case on my Ubuntu install where is return '/').

I have tested on Ubuntu 9 (that the changes wok) and MacOS X (that nothing is broken).
I would appreciate if you could also test with the trunk. Then I will backport to 1.1.0 and close the bug.

comment:5 by criezy, 10 years ago

Owner: set to criezy

comment:6 by salty-horse, 10 years ago

Is there a reason to keep wxStandardPaths::Get().GetResourcesDir() in the code?

APP_MEDIA_PATH is ALWAYS defined by configure, so looking for files before that seems odd (and may cause weird problems if you have old files in the GetResourceDir())

comment:7 by criezy, 10 years ago

The reason why I keep wxStandardPaths::Get().GetResourcesDir() is only for MacOS X. APP_MEDIA_PATH is not defined on platforms where you don't use configure (Windows?) and is defined but not correct on MacOS X (when you create a bundle).

I could I suppose use some defines to use it only on MacOS (and Windows?) since it is not needed on other Unix/Linux where APP_MEDIA_PATH should be correct. (and could indeed cause weird problems).

comment:8 by salty-horse, 10 years ago

#ifdefs are a good idea. A comment that explains which is used on which platform/scenario would also be welcome.

comment:9 by fingolfin, 10 years ago

Actually, I would recommend always using wxStandardPaths::Get().GetResourcesDir() and not limiting it to Mac OS X or Windows. There might be other OSes using this. Artificially not using a portability API for what it was meant to seem to me worse than just always using it (the risk of "old files" is rather low, IMO; we can reconsider this if we ever get bug reports caused by this, right?)

comment:10 by fingolfin, 10 years ago

BTW, there are Linux distros like GoboLinux which also use a different system layout, and I believe one can move around application "bundles" there, too. I have no idea if their wxWindows port supports that (or even if I am actually right on my claim -- I only once played briefly with this distro out of curiosity), but this underlines that you should be careful making assumptions about when it's "right" to use this API and when not.

comment:11 by criezy, 10 years ago

I don't know GoboLinux but I agree it is a bad idea to assume all Linux distros out there use the same system layout. Initially I added the APP_MEDIA_PATH case because wxStandardPaths::Get().GetResourcesDir() does not always return the expected directory (on my Ubuntu it returns "/"). So it was meant as a fallback in case wxStandardPaths::Get().GetResourcesDir() does not work as expected.

I have changed again the code to always try wxStandardPaths::Get().GetResourcesDir() first and then a number of fallback cases to optimise the chances to find the files.

This has also been backported to the 1.1.0 branch.

comment:12 by criezy, 10 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.