"savefiles" is not created automatically

[Bodasey]

Observed behaviour

The directory "savefiles" is not created automatically The load/save game routine crashes the game

Steps to reproduce

rename the directory "savefiles"

My pioneer version (and OS):

actual master on SuSE Tumbleweed

My output.txt (required) and game save (optional, but recommended)

When saving: Info: Error picking auto-save Info: Created shader gassphere_base (address=0x29d82b30) Info: Created shader geosphere_sky (address=0x26ad5100) Info: Created shader planetrings (address=0x2867af10) Info: Created shader geosphere_terrain (address=0x29e79df0) Info: Error picking auto-save Info: Error picking auto-save Info: Error picking auto-save Info: Error picking auto-save Info: Error picking auto-save Info: Error picking auto-save Info: Error picking auto-save Info: Error picking auto-save Info: Error picking auto-save Warning: Caught error in Lua UI code: [T] pigui/modules/saveloadgame.lua:281: bad argument #1 to 'sort' (table expected, got string) libpng warning: iCCP: known incorrect sRGB profile Info: WARNING: texture 'icons/object_star_m.png' is not power-of-two and may not display correctly Info: WARNING: texture 'icons/object_planet_ice.png' is not power-of-two and may not display correctly Info: WARNING: texture 'icons/object_orbital_starport.png' is not power-of-two and may not display correctly Info: StarSystemCache: misses: 595, slave hits: 0, master hits: 235799 Info: SectorCache: misses: 100, slave hits: 5057, master hits: 161920 Info: Pi shutting down. Info: StarSystemCache: misses: 0, slave hits: 0, master hits: 0 Info: SectorCache: misses: 0, slave hits: 0, master hits: 0

when loading Fatal: [T] pigui/modules/saveloadgame.lua:281: bad argument #1 to 'sort' (table expected, got string) stack traceback: [C]: in function 'sort' [T] pigui/modules/saveloadgame.lua:281: in function 'makeFileList' [T] pigui/modules/saveloadgame.lua:355: in function '?' [T] pigui/libs/module.lua:43: in function 'update' [T] pigui/modules/saveloadgame.lua:336: in function 'update' [T] pigui/libs/modal-win.lua:72: in function 'drawModals' [T] pigui/libs/modal-win.lua:96: in function 'draw' [T] pigui/views/mainmenu.lua:120: in function 'callModules' [T] pigui/views/mainmenu.lua:197: in function <[T] pigui/views/mainmenu.lua:126>

[mwerle]

Ditto on Debian using the "AppImage" (20240710).

When trying to load/save, the log is spammed with:

"Info: Error: 'user://savefiles' is not a directory"

No other error or informational output was generated.

Furthermore, for normal Linux users, this log information is completely useless as "user://" is a URL, not a filesystem path. I did, eventually, figure it out as in "Options" there is a "User Folder" button, which confused me for a long time as it was not possible to edit this; it turns out it's just informational (/home/micha/.pioneer in my case).

Manually creating /home/micha/.pioneer/savefiles resolved the issue for me and allowed me to save/load the game.

Firstly, the bug needs to be fixed, obviously, secondly, it would be useful to improve the error logging, and, finally, it should be possible to select the "User Folder" or, at least, default it to the platform-appropriate location. In the case of GNU/Linux, I would suggest that this should probably be "${HOME}/.config/pioneer" as per the XDG recommended layout.

[mwerle]

.. I had a quick look at the sources, but could not figure it out. There appears to be a call to create the savefiles directory in Game.cpp::SaveGame, but the error is actually raised inside a LUA module which does not attempt to create the directory. I have no idea how the C++ and LUA interact.

Furthermore, there does appear to be code to use the XDG location on Posix systems; no idea why in my case it chose to use $HOME/.pioneer instead.

Kudos to the developers, the game compiled and ran without any issues on my machine, unlike most non-trivial open-source projects which require significant faffing about before it works.

[impaktor]

there does appear to be code to use the XDG location on Posix systems; no idea why in my case it chose to use $HOME/.pioneer instead.

@mwerle Check your XDG_DATA_HOME.

I have no idea how the C++ and LUA interact.

Lua files reside in data/ (probably data/pigui/ for this case), these call C++ modules in src/lua/*cpp (see bottom of each file for where C++ functions are mapped to lua method names), and these files expose the c++ side of the game to the lua engine. We have disabled making file system changes from lua (e.g. to prevent someone from making a pioneer mod calling "rm -rf /"), thus writing to filesystem is (was?) done in C++ and compiled into the game, then called from lua. It's possible lua might be allowed to write directly only to the pioneer user's folder, I don't remember right now.

Either way, sounds like a good first code contribution if you want to fix the issue.

Kudos to the developers, the game compiled and ran without any issues on my machine

All development is done on linux

[mwerle]

there does appear to be code to use the XDG location on Posix systems; no idea why in my case it chose to use $HOME/.pioneer instead.

@mwerle Check your XDG_DATA_HOME.

So the "AppImage" version of Pioneer saves in $HOME/.pioneer; it seems it can't figure out any XDG stuff even though the code should default to ".local/share/pioneer". My self-compiled copy uses ".local/share/pioneer" - I was expecting it to use ".config/pioneer", but I guess the game doesn't distinguish between settings (aka configuration) and data (ie, savefiles).

Either way, sounds like a good first code contribution if you want to fix the issue.

Thank you. I'll see if I can muddle through.. I did find the relevant bits of code, but it's a really good pointer that the LUA calls into the C++ and the C++ is what does the actual work. From a safety perspective, that's a really good design to prevent the LUA scripts from doing anything potentially dangerous to the users' machine!

[mwerle]

... and the fix turned out to be trivial: (the hint was in the actual error message, which I should have concentrated on from the start..)

PR : https://github.com/pioneerspacesim/pioneer/pull/5919

--- a/data/pigui/modules/saveloadgame.lua
+++ b/data/pigui/modules/saveloadgame.lua
@@ -274,10 +274,10 @@ function SaveLoadWindow:makeFileList()
        if not ok then
                Notification.add(Notification.Type.Error, lui.COULD_NOT_LOAD_SAVE_FILES, files --[[@as string]])
                self.files = {}
+       else
+               self.files = files
        end

-       self.files = files
-
        table.sort(self.files, function(a, b)
                return a.mtime.timestamp > b.mtime.timestamp
        end)

The bug was introduced in commit 7b6a79700682124d29351fb83562778eaf5a2da4 which refactored the loading/saving interface.

I guess there's not much in the way of unit-tests for this project?

[impaktor]

@mwerle Cool. ~Now open a pull request for that change.~ (Edit: I see you've already done it)

I guess there's not much in the way of unit-tests for this project?

We (each) have one that uses bi-occular input to feed the code to biological hardware / wetware, however, it first must parse and compile this source, which is a slow process: https://dev.pioneerspacesim.net/contribute/coding-conventions