#4007 closed defect (fixed)
GUI: theme detection
| Reported by: | SF/begasus | Owned by: | lordhoto |
|---|---|---|---|
| Priority: | normal | Component: | GUI |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
After I compiled the source and do a make install, and then run scummvm it doesn't seem to detect scummmodern.zip, I can set it via options and then it gets detected.. but it should be detected on startup. Came across this behaviour in BeOS's and again when looking it up on my linux box. Here is the output from Terminal when trying to launch scummvm:
~/develop/scummvm$ scummvm WARNING: File::open: 'scummmodern.zip' does not exist! WARNING: Could not parse custom theme 'scummmodern.zip'. Falling back to default theme!
~/develop/scummvm$ svn up At revision 34891.
Ticket imported from: #2219605. Ticket imported from: bugs/4007.
Change History (20)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
Actually, the value of gui_theme in the .ini is supposed to be a full path these days. The current default value "scummmodern.zip" won't work for that reason, but that's expected. If you want a custom theme, you'll have to choose it via the theme browser (options dialog -> misc tab -> select a theme).
So, one way to resolve this is to remove that default, then close this issue as "works as expected".
Alternatively, we could add some code that acts as fallback if the specified gui_theme is not a valid path: Instead, scan for all themes in themepath etc. (by reusing the code from the themebrowser). Then, match that against the gui_theme value
For the latter, two possibilities come to mind: Assume gui_theme contains the file/dir name of the theme, so match it against the path of all found themes; or assume it is the name of the theme, so match it against the name of all found themes.
comment:3 by , 16 years ago
I'm with Max and the fallback code which looks for the specified theme file in all the theme folders. I don't really like the idea of removing a the Modern theme as the default theme and having to choose it manually.
I'll make a patch and put it up for revision this afternoon. I'll make sure to handle the case when someone updates from an old version of ScummVM and his gui_theme is still pointing to the old Modern.ini (this was requested a while ago on IRC).
comment:4 by , 16 years ago
| Summary: | theme detection → GUI: theme detection |
|---|
comment:5 by , 16 years ago
| Owner: | set to |
|---|
comment:6 by , 16 years ago
Ok, I gave this some more thought before sitting down and writing the fallback code.
The thing is, what's really the point on some fallback code which is called when the gui_theme var is not set to a real path but to a single file? The theme manager always saves the current theme with its full path, so the only time the fallback is really needed is for the first time ScummVM is ran, because it doesn't point to the modern theme yet. Wouldn't it be better to remove the default value for gui_theme and make the code set it the first time its run? Can this be done at all?
comment:7 by , 16 years ago
The registerDefault call serves only one purpose: A single invocation of ConfigMan.registerDefault("foo", defaultVal); allows us to replace code like if (ConfigMan.hasKey("foo")) val = ConfigMan.getKey("foo"); else val = defaultVal; by a simple val = ConfigMan.getKey("foo"); everywhere. Not only is this shorter, but it's now also less likely to first check for the presence of the key, and we don't plaster the same default value over multiple spots.
But of course, if you know for sure that a config key is only ever used in 2-3 places you are watching closely, you can easily work without registering a default.
Regarding your other points: Yeah, I'd prefer to have the modern theme as default, at least on desktop machines. The "problem" is just this: We don't know its location on the disk. Of course, we can find out using the code in the themebrowser. But if we implement fallback code which converts old-style "gui_theme" values to paths automatically (by reusing the code from themebrowser), then why add yet another code path if a simple ConfigMan.registerDefault("gui_theme", "scummmodern.zip"); does the same?
comment:8 by , 16 years ago
May i add that i get similar behaviour on AmigaOS4 SDL backend
ScummVM 0.13.0svn (Nov 12 2008 14:36:14) Features compiled in: Vorbis FLAC MP3 zLib MPEG2
File <File Stream>, line 55: <bitmap filename = 'logo.bmp'/> <bitmap filename = 'cursor.bmp'/>
Parser error: Error loading Bitmap file 'logo.bmp'
WARNING: Failed to parse STX file 'scummmodern_gfx.stx'! WARNING: Could not parse custom theme 'Games:ScummVM/themes/scummmodern.zip'. Falling back to default theme!
I have my themes in a special theme folder saved in thepath options
comment:9 by , 16 years ago
A comment also from my side: Thats not only a BeOS/AmigaOS problem. The Windows version takes only the builtin classic theme when trying "options dialog -> misc tab -> select a theme". All other themes like "scummmodern.zip" are ignored although they are listed in the launcher options. In my case the themes are in the same folder as ScummVM, so no extra folder is used and it doesn´t make a difference if I set up a theme path or not. If I remove the path in the scummvm.ini for e.g.: "gui_theme=C:\Program Files\ScummVM\scummmodern.zip" to "gui_theme=scummmodern.zip" then the theme will be loaded. So it is in general not possible using the launcher to set up a theme.
comment:10 by , 16 years ago
I get the same error as raziel_ with Windows svn build. It doesn't read the zip file. I have to uncompress to scummmodern directory to be able to choose it.
File <File Stream>, line 32: <def var = 'WidgetSize' value = 'kBigWidgetSize' />
<def var = 'Padding.Bottom' value = '16' />
Parser error: Invalid definition for 'Globals.WidgetSize'.
WARNING: Failed to parse STX file 'C:\Program Files\ScummVM\scummmodern\scummmod ern_layout.stx'! WARNING: Could not parse custom theme 'C:\Program Files\ScummVM\scummmodern\'. F alling back to default theme!
With latest svn build (15 november)
comment:12 by , 16 years ago
Ups, you were right, You can't select modern, but classic (not builtin) yes ¿?
comment:13 by , 16 years ago
Yes, you can only select the "Classic Theme" regardless if it is built-in or as external zip archive. I don´t know if it has relevance but there might be a context to another bug I found for the WinCE version, which does crash when a theme is changed from classic to modern but not the other way round when changing from a modern theme to classic theme. It has ID 1976799: https://sourceforge.net/tracker2/?func=detail&aid=1976799&group_id=37116&atid=418820
comment:15 by , 16 years ago
Same for the PC version (SVN rev. 35203 MSYS/MinGW compiled). Seems to be working now here.
comment:16 by , 16 years ago
| Owner: | changed from to |
|---|---|
| Resolution: | → fixed |
| Status: | new → pending |
comment:18 by , 16 years ago
| Status: | pending → closed |
|---|
comment:19 by , 16 years ago
This Tracker item was closed automatically by the system. It was previously set to a Pending status, and the original submitter did not respond within 14 days (the time period specified by the administrator of this Tracker).
comment:20 by , 6 years ago
| Component: | → GUI |
|---|
The problem is that openThemeXML creates an FSNode("scummmodern.zip"), which only looks in the current directory. Using File::open() to locate this .zip with SearchMan will provide a File object, but ZipArchive has no constructor from that. (It only has a constructor from FSNode, effectively.)
I think we might want to change ZipArchive to accept a File (or even a SeekableReadStream?) as input.