Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: screenshot minimap is stored incorrect place in dedicated server #12550

Closed
telk5093 opened this issue Apr 20, 2024 · 8 comments · Fixed by #12679
Closed

[Bug]: screenshot minimap is stored incorrect place in dedicated server #12550

telk5093 opened this issue Apr 20, 2024 · 8 comments · Fixed by #12679

Comments

@telk5093
Copy link
Contributor

Version of OpenTTD

14.0, Ubuntu 22.04

Expected result

A minimap screenshot should be generated in /screenshot directory of where openttd is.
For instance, if openttd is in /home/ottd/, the screenshot file should be in /home/ottd/screenshot

Actual result

The screenshot file generated in ~/.local/share/openttd/screenshot/
It is a something like re-report of #9231

Steps to reproduce

  1. openttd -D -c /some/path/openttd.cfg
  2. Run screenshot minimap test
  3. test.png is generated in ~/.local/share/openttd/screenshot, not in /some/path/screenshot.
@glx22
Copy link
Contributor

glx22 commented Apr 21, 2024

From https://github.com/OpenTTD/OpenTTD/blob/master/docs/directory_structure.md, Savegames will be relative to the config file only if there is no save/ directory in paths with higher priority than the config file path, but autosaves and screenshots will always be relative to the config file.

So the screenshot part is "broken" and according to discord, the autosave part has the same issue.

@telk5093 telk5093 changed the title [Bug]: screenshot minimap does not generate files in dedicated server again [Bug]: screenshot minimap is stored incorrect place in dedicated server Apr 21, 2024
@PeterN
Copy link
Member

PeterN commented May 3, 2024

Can't reproduce on my dedicated server. list_dirs save (etc) all point to the path with the configuration in it, not ~/.local/share/openttd/

@telk5093
Copy link
Contributor Author

telk5093 commented May 5, 2024

For notes:
I've installed my dedicated server by getting the zip archive via wget, stored in /home/ottd/stable/ and with a custom config /home/ottd/stable/openttd.stable.cfg

Here's my output of list_dirs.

list_dirs screenshot
/home/telk/.local/share/openttd/screenshot/ [ok]
list_dirs save
/home/telk/.local/share/openttd/save/ [ok]
list_dirs newgrf
/home/telk/.local/share/openttd/newgrf/ [ok]
/home/telk/.openttd/newgrf/ [not found]
/home/ottd/stable/newgrf/ [ok]
/usr/local/share/games/openttd/newgrf/ [not found]
/home/ottd/stable/content_download/newgrf/ [ok]
/home/telk/.openttd/content_download/newgrf/ [not found]
/home/telk/.local/share/openttd/content_download/newgrf/ [ok]

@rubidium42
Copy link
Contributor

The bug report itself is a bit misleading, I think.

You are talking about openttd -D -c /some/path/openttd.cfg, but you mean /some/path/openttd -D -c /some/path/openttd.cfg. As the "issue" only happens when the folder with the executable is the same as the folder with the configuration file.

In any case, it got broken by #11431. I've got no idea how that fixed the Emscripten stuff, or what was wrong with it. @TrueBrain, can you shed some light on that?

@TrueBrain
Copy link
Member

I think #11431 covered it pretty well, looking at the commit that it fixes.

cda6f24 doesn't allow duplicated folders in our searchpath. Which broke emscripten.

Some of our code ignores the SP_WORKING_DIR for some actions, which means that if, for example, your SP_BINARY_DIR is the same as your SP_WORKING_DIR, neither is scanned.

Wording it differently, say you have a situation where SP_BINARY_DIR is also SP_WORKING_DIR. Because of the above commit, SP_BINARY_DIR was removed. With #11431, SP_WORKING_DIR instead is removed. Because SP_WORKING_DIR isn't always searched in all cases.
In either case, the working directory is on the search path.

In that sense, the actual issue was introduced by #11363, adjusted in #11431 to prioritize SP_BINARY_DIR instead of SP_WORKING_DIR.

PS: I did not read this ticket, just answered what #11431 does :)

@rubidium42
Copy link
Contributor

rubidium42 commented May 13, 2024

So I guess this essentially this boils down to choose at most two of:

  1. have screenshots/saves be placed in the expected location when configuration and binary are in the same folder.
  2. have working preview builds.
  3. perform no unneeded/duplicate scanning work.

I got no clue what exactly broke (only?) the preview builds, so I got also no clue how to untangle that problem. For the screenshots the SP_WORKING_DIR must be set (or SP_BINARY_DIR must be set with an appropriately high priority), but checking SP_BINARY_DIR is the same and increasing its priority seems like a very hacky and fragile solution.

@TrueBrain
Copy link
Member

TrueBrain commented May 13, 2024

Be mindfull SP_BINARY_DIR is not given priority with #11431 ; SP_WORKING_DIR is given no priority (as normally we don't save files in the working directory; this situation excluded) when it is a duplicate of any of the other SPs.

@rubidium42
Copy link
Contributor

SP_WORKING_DIR has the highest priority (it is literally SP_FIRST_DIR). It's changed to the location of the configuration file if that's given.

rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 13, 2024
rubidium42 added a commit to rubidium42/OpenTTD that referenced this issue May 13, 2024
…nary and configuration are in the same folder
rubidium42 added a commit that referenced this issue May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants