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

Find solution for the problem of user config "secretly" overriding default cfg #975

Closed
ghost opened this issue Oct 24, 2019 · 37 comments
Closed
Assignees
Labels
context: user config res: in consideration type: enhancement a suggestion or necessity to have something improved
Milestone

Comments

@ghost
Copy link

ghost commented Oct 24, 2019

This is written after this: #789, but the problem exists since introduction of a user config which is written in a "safe writeable" directory (currently: in the saves dir), and somewhat worsened when engine began to save particular options upon exit (options that could be changed at runtime, such as translation for instance).

The problem is this: unless user is aware that this extra config file is saved, he or she may be confused when trying to change a config option in the file inside game dir only to see that it does not work. This is specifically essential on Linux, where we do not support builtin setup program.
Even if user is aware of that, it may be simply tiresome to find config file for particular game every time.

Note that since AGS 3.5.0 engine can tell where configs are stored if you run it like ags --tell-configpath or print current effective config if you do ags --tell-configm which may help diagnosing the situation. But that don't change the fact that you still have to keep this in mind and search for the user config.

So, some kind of solution is needed to ease this situation for developers and players. I'll leave this ticket open for suggestions.

See Also: #961

@i30817
Copy link

i30817 commented Nov 14, 2019

I don't think there is a good solution in the absence of a config program for naive users. The main problem is that it's the unaware user that will have this problem and making them 'aware' after the fact when AGS runs with a timestamp of option override detection is both intrusive at best and most likely a false positive from the game distribution if it wasn't the user itself screwing up.

If you do want to get something that warns the user and not prevent them from screwing up, a text overlay for 5 seconds after ags starts that warns the main dir cfg is newer than the replacement one is maybe appropriate but it'd probably be very annoying on the small edits where it doesn't matter. It may be better to add a 'and' condition:

main dir cfg timestamp newer and option exists in the replacement cfg that replaces existing option in the main dir cfg but even that is not really 100% false positive free because the second condition might have been edited by the game itself changing language settings and be unrelated to the user change or something like that.

In short it may be better to punt and distribute a small 'config' program so prevention of naive users and simply warn on the console that a replacement cfg has a older timestamp than the original cfg and that maybe they want to delete (if they want the change to be the new 'default') or edit the 'override' file instead, for the less naive but more arrogant users.

@legluondunet
Copy link

legluondunet commented Jan 24, 2020

same issue here:

$ ./ags64 --tell-configpath
[config-path]
default = /home/legluondunet/Bureau/tmp/ags/ags_3.5.0.23_linux/data/acsetup.cfg
global = /home/legluondunet/.local/share/ags/acsetup.cfg
user = /home/legluondunet/.local/share/ags/Maniac Mansion Deluxe/acsetup.cfg

but in fact the global (or user?) config file takes control over the default one in game path.
Could it be possible to ask AGS to ignore other config files when there is one in the game data path?

@ivan-mogilko ivan-mogilko added this to the 3.5.1 milestone Jan 24, 2020
@ericoporto
Copy link
Member

@legluondunet I am making a graphical setup program, the idea is that a developer could ship it along with their game, if they choose to, while there's no default one provided by the AGS Engine - I lack in skills to figure out and code a proper one in the engine itself.

@ghost
Copy link
Author

ghost commented Jan 24, 2020

Is it not possible to ask ags to ignore other config files when when there is one in the game data path?

It should be possible, #961

@ivan-mogilko
Copy link
Contributor

Posted by @legluondunet in #1163 (closed as duplicated and moved here)

I'm writing a game script on Lutris (games platform for Linux) which will use AGS.
I need support about an issue.
I put a config file in main game folder, acsetup.cfg.
But when AGS launch it copies this config file in /home/your-username/.local/share/ags/your_game/.
Then AGS only take care of the cfg file located in /home/your-username/.local/share/ags/your_game/.
Modifications done on acsetup.cfg in main game folder are no more taken into account.
Is there a way to tell AGS to not copy config file in user profile and just take in account the config file in main game folder?
Our installs are locale, we don't want to use user profile folder, Lutris should manage AGS option in game folder (or other place we could choose).

I guess the point is that you should be able to also tell where to put the config exactly (not just switch between game folder/default system location)

@legluondunet
Copy link

@ericoporto , I see this option more like a command line option that will let users choice which config file will override others, will be nice.

example:
ags --config=/path/of/your/choice/acsetup.cfg

@legluondunet
Copy link

legluondunet commented Jan 9, 2021

The actual system could be confusing for users. First time I used AGS, I didn't understand at first why my changes in config file located in main game folder was not taken in account. Until I searched on the web and found that this file was then copied in profile folder and overridden the one in game folder.

@ericoporto
Copy link
Member

ericoporto commented Jan 9, 2021

@legluondunet There's currently a single config file that overrides all config files, depending how you are writing the Lutris config you could just use it instead.

$XDG_DATA_HOME/ags/acsetup.cfg

https://adventuregamestudio.github.io/ags-manual/RuntimeEngine.html


AGS needs a graphical interface for configuration.

@legluondunet
Copy link

legluondunet commented Jan 9, 2021

I'm not against graphical interface at all but it is heavy to develop and maintain, even more for a multiplatform project and when it doesn't work, it is still nice to have possibility to manage options with a command line. A command line is far easier and quicker to port in case of a Multi-Platform project.
With command lines options, AGS could be efficiently used in a frontend or in a script, like others popular existing game engines (GZDOOM, most emulators...).
Secondly, about game settings location, in my last message I shared my experince the first times I used AGS, I modified config file in main game folder and the changes was not taken in account, I conclue the actual config system could give confusion to users. To my point of view, if one config file should overriden others, it should be the one in the game folder. If I was a AGS game developper, when I distribute a AGS game, I would appreciate to control game options. If I changed them and distribute an update to my users, the new changes will not not be takken in account as there is already a config file installed in user profile.
Other example, when users change computer, they will lose config file in userprofile, but not the one in game folder.
To finish, add a command line option to specify a config file is a current feature for a game engine and does not need lot of code to implement it. If it can be usefull for some users.

@rofl0r
Copy link
Contributor

rofl0r commented Jan 9, 2021

my personal preference would be to always use game directory for config/savegames, unless user specifies a command line option/environment variable to use XDG path instead, or a platform is targeted that is known read-only.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jan 9, 2021

@ericoporto

AGS needs a graphical interface for configuration.

But this is mostly irrelevant to the problem. GUI setup is always optional and preferably uses already available engine features, that may be accessible from command line or through a config file. These are two ways of doing same thing, which may co-exist. But engine's functionality should not depend on availability of a GUI.

@ivan-mogilko
Copy link
Contributor

legluondunet's problem may be actually similar to #1156, as both originate with engine writing user config on quit.
Maybe that was not the best idea to do unconditionally. So, if I'd undo that the problem will be solved temporarily or in some circumstances.

It may resurface though if a setup tool was used and wrote file in different dir again, or gamedevs would be given a control over standard config in which they may write options (such as last language, volume, etc). This is why I believe this problem needs a more general solution.

@AlanDrake
Copy link
Contributor

AlanDrake commented Jan 10, 2021

Why not bundle the default setup in the exe and avoid creating acsetup.cfg in the game folder?
Perhaps if it were named default.cfg and had comment lines warning users that this is not the file they want to edit would have been a better starting point, of course everything is 20/20 in hindsight,

I would avoid creating anything mutable in the game folder, saves should default to a "saves" subfolder, or the alternate userprofile folder, depending how the game was configured in the editor.
The game should never change files on the root game directory, even though it should keep searching inside it as a fallback.

This way you can always safeguard defaults, the user saves are always on a separate folder and can be wiped without danger.
Also games that provide level editors and stuff can keep the original states in the game folder, while the user modifications are duped in their save folder to override the base stuff.

There only need be one single option: "Prefer saving in the user profile folder" True/False. You may even get rid of the ability to customize which folder.

EDIT: also could the executable itself be used as a proxy to change and persist options? Letting it taking care of saving wherever it is it's supposed to save? A lot of problem could be avoided with that, and would help making third party setup GUIs easier because they don't need to know the logic of the save folder resolutions, they just need to silently call the executable with the right parameters.

@i30817
Copy link

i30817 commented Jan 10, 2021

Before you get into many divergent solutions, please don't break what's there. People - aka me - use the config file in the main directory to save 'defaults' and sometimes those config files are not writable (because the game is on a read only dir).

The current situation is fine if the people who don't know what's happening remember to use a program to deal with this conditional for them, so just write that program or something.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Jan 10, 2021

@AlanDrake

Perhaps if it were named default.cfg and had comment lines warning users

True, one of the ideas I am currently considering: split acsetup.cfg into default.cfg and user.cfg. This way even if user.cfg is written into same dir (by a person, setup application or engine itself) - it will be clear what is what. In my thinking this approach will also safeguard user settings in case they install an update over same dir if that update include default config too for some reason.

Why not bundle the default setup in the exe and avoid creating acsetup.cfg in the game folder?

That is actually possible, and afaik some engines have configs in their "paks" among the assets. This may prevent the occasional loss of config data precisely crafted by the author :-). As well as it is possible to have a identically named config file in the game dir that will be overriding the one in game files according to the general asset override rule.
Idk if this is worth it in current situation though, as we dont have too much settings in config at the moment, and engine seem to use good defaults to fallback to.

A lot of problem could be avoided with that, and would help making third party setup GUIs easier because they don't need to know the logic of the save folder resolutions, they just need to silently call the executable with the right parameters.

This solution already exists: see --tell command line arguments, they output locations which engine considers for the particular game files according to all the rules.


@morganwillcock
Elsewhere you wrote:

On Windows this could be pretty confusing, because unprivileged writes to a privileged location will be silently redirected to a per user location (within the user profile). If this feature is added, to work as intended it would need to explicitly bypass the redirection, in-case the game has been copied to a location where the user cannot write, and also gracefully handle being denied access.

Could you please give a link where I can read about this?
There are two questions here:

  1. may this be detected by writing a test file and then trying out if it actually exists where we wanted it to be?
  2. if we cannot write to the game dir at all, then we will have to write in user data folder anyway, so would be let system do it for us or do explicitly?

@morganwillcock
Copy link
Member

I'm not sure where official documentation for it is, I think technically it is part of the UAC mechanism. You should be able to observe it in action by having a process write something or edit something in 'Program Files' and see the result appear inside the user profile at 'AppData\Local\VirtualStore'.

  1. may this be detected by writing a test file and then trying out if it actually exists where we wanted it to be?

No, because it is effectively an overlay of the original file. The writing process doesn't know that it didn't edit the real thing. When you read it you are transparently reading the copy, not the original. If you must read/write to the real location you must configure the process to prompt for elevation with a UAC prompt, and for a game engine this is not appropriate.

  1. if we cannot write to the game dir at all, then we will have to write in user data folder anyway, so would be let system do it for us or do explicitly?

I don't think it is a good idea to rely on this mechanism to store data, because:

  • this is really just a compatibility shim for applications which aren't UAC compliant
  • part of the argument for writing the config to the game directory is that it 'makes a mess' of people's computers, and this would actually be writing data into locations that peoples aren't even aware of, which seems worse that actually using the locations that are meant for application data

Fundamentally, I don't think people are going to agree on an approach because the use cases are so different.

From a game author's perspective I would agree that setup data that comes with the game should be immutable and anything else be a permitted adjustment of that, potentially from a patching perspective the game author may need to override or reset a user setting. The computer is not under their control so any actions need to follow platform guidelines and there should be an acceptance that the game installation may have multiple users (potentially even concurrently).

From the perspective of someone wanting to run old games they will want to override and adjust everything, using mechanisms that are the most convenient, all running within the context of a single user, on a system they likely manage themselves.

My preference would always be to the first group since:

  • AGS is game development tool
  • Platform guidelines should be followed so AGS games operate expectedly when shipped

As a workaround for the second group, perhaps an explicit use this configuration file for everything and ignore everything else option would suffice: something that has to be manually enabled but is not beholden to anyone's opinions or any platform rules. The recent suggestion for a command line switch to do it didn't seem a bad idea to me, or maybe check for an environment variable - given the context of the problem it would probably be best to not involve any files at all in turning such an option on.

@legluondunet
Copy link

legluondunet commented Apr 11, 2021

Hello, is there some progress about this issue?
What about a command line option to precise another config dir than the default one to store game config file and savefiles?
This solution seems to suit most contributors from this discussion.
What is the blocking element?

@rofl0r
Copy link
Contributor

rofl0r commented Apr 11, 2021

in order to make this simple to implement, a command line option could just override all other locations - so if you use ags --confdir=. ags would use ./acsetup.cfg as the one and only config file, without interference from other locations. that way we don't need to shoehorn even more complicated logic into the current system.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Apr 11, 2021

Sorry, I was going to look into this very soon. It was planned to be part of 3.5.1 release.

There are other related things that I'm currently considering.

Regarding command line option, that may work of course, but unfortunately this is likely not going to be convenient for many users who do not use command line to run the games, including ones on Windows.
So, we can likely add command for this, but then there has to be something else too.

PS. Guess we may start with command line option and then when it's done consider if there may be an alternative to trigger same option.

@ivan-mogilko
Copy link
Contributor

In regards to command line option, there's also this question.
Currently there are 2 special locations ags game can use:

  • save data dir, also contains user config for historical reasons
  • shared data dir, where game saves game files supposedly shared between all users

These two may currently be set in user config (but that leaves user config location unaffected of course) with "user_data_dir" and "shared_data_dir" options respectively.

If there's a command line option to define config location, would it define only config itself, or also these two?

@rofl0r
Copy link
Contributor

rofl0r commented Apr 11, 2021

If there's a command line option to define config location, would it define only config itself, or also these two?

depends on the naming. if the option is called configfilepath, then it would be unexpected to also influence other paths.

if it would be called --basepath or similar, then it could affect all of the 3.

@legluondunet
Copy link

legluondunet commented Apr 11, 2021

shared data dir, where game saves game files supposedly shared between all users

what are this files? All the game needed files are not in the game folder?

if it would be called --basepath or similar, then it could affect all of the 3.

We could effectively dissociate saves and config paths.
--savepath=
--configpath=
--basepath=

but unfortunately this is likely not going to be convenient for many users who do not use command line to run the games, including ones on Windows.

The idea is not to force all users to use AGS in command line but to implement possibility for advanced users or frontend devs to change AGS default path config/save dir.
This games apps already have a lot of command line options and you can define their config/saves/... path:
MAME
ScummVM
GZDoom
Dosbox
...
Very practice to integrate them in a frontend or other gaming platform like Lutris.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Apr 11, 2021

shared data dir, where game saves game files supposedly shared between all users

what are this files?

This is for game author to decide. It's an opportunity, not all the games use it, but some do. E.g. they may save some generated data that is required for the game to work regardless of player and their progress.

The idea is not to force all users to use AGS in command line but to implement possibility for advanced users or frontend dev to change AGS default path config/save dir.

I understand that, but there's also a request from regular users to have this option. So this is something I'll have to consider separately after the command line option is implemented.

@ivan-mogilko
Copy link
Contributor

Opened preliminary pr: #1252

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Apr 15, 2021

This ticket should probably be closed, because it may become a source of confusion.

Since AGS 3.5.0.31 engine no longer creates or writes user config on exit. This means that the only way a user config may appear in $XDG_DATA_HOME/... on Linux is either a) created by user's hand, b) created by some setup application. In both cases a user is responsible or at least aware of what's happening.
The situation is similar on Windows.

Above resolves a problem users had when they were editing default config by hand or by some custom tool/app, and then found that the engine uses different options for unknown reasons.

This does not resolve a problem with users who want our embedded setup app on Windows write in a game dir instead of a user folder. There's also still a remaining question of how to deal with this situation if the engine will be writing to user config again at some point in the future (perhaps, if we allow to do so by script command).
I still like to address these problems, but don't want to keep this ticket open, as it discusses number of different use cases.

IMO latest @morganwillcock 's comment may be used as a reference for further changes:
#975 (comment)

@i30817
Copy link

i30817 commented Apr 16, 2021

Hey just a question. Since the engine no longer writers the duplicate config, what happens if the main config is read only and the game tries to write? I have games in a read only medium (squash filesystem). Just ignore the changes right? I can live with that.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Apr 16, 2021

what happens if the main config is read only and the game tries to write?

First of all, game directory itself is forbidden to be explicitly written into with File script commands. If the game tries to open a file for writing in a game dir using $INSTALLDIR$ tag, then File.Open will fail and return null pointer (I don't remember if it also displays error or not). It does not matter whether directory is writeable or not.

The only way game may end up writing anything in the game dir is when user remaps e.g. savedir in there (using "user_data_dir" config option). Then game writing to $SAVEGAMEDIR$ will actually be writing into game dir (without knowing about it). Of course if the location is not writeable the File.Open will fail.

@i30817
Copy link

i30817 commented Apr 17, 2021

As long as it doesn't crash and burn i'm fine with it. If it depends on the game devs to check results otherwise it crashes, uhoh...

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Apr 17, 2021

If it depends on the game devs to check results otherwise it crashes, uhoh...

Of course, it depends on whether devs checked the returned File pointer. If they did not, the game will crash with null pointer error.

But then again, what situation are we discussing? Like I said, writing in game dir is forbidden in AGS, this rule exists for several years now.

@i30817
Copy link

i30817 commented Apr 17, 2021

I've seen several games that had a inbuilt 'language changer'.

I also place my games on a read only squash.

So i'm worried that if the engine is not creating writable cfg files in $XDG_DATA_HOME, it will crash when a game tries to acquire a writable handle to the cfg file, fails it on the game dir, fails it on $XDG_DATA_HOME because it wasn't created by the engine and returns a NULL to the game engine to 'set the language'.

Though thinking about this, that fallback is usually open or create, so maybe it's not so bad.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Apr 17, 2021

I've seen several games that had a inbuilt 'language changer'.
So i'm worried that if the engine is not creating writable cfg files in $XDG_DATA_HOME, it will crash when a game tries to acquire a writable handle to the cfg file, fails it on the game dir, fails it on $XDG_DATA_HOME because it wasn't created by the engine and returns a NULL to the game engine to 'set the language'.

When game asks to open a file for writing engine will open the file regardless of whether it existed before or not. It will create one if necessary, - so long as the path is writeable.

If file did not exist, and game opened it for reading - then it will return a null pointer, but the game script is supposed to work around that.

After all, game may be run without launching setup program first, in which case there would not be any user configs created at all. A sane game developer should take that into account.

@i30817
Copy link

i30817 commented Apr 17, 2021

Well i'm kind of worried about something else now that you explained that.

So my original fear was that those games would be writing their cfg and screwing it up now that ending up with a null pointer was more probable. But then i remembered that 'the engine previously wrote the cfg file on exit'.

So the risk, which i think might happen in the future, is that developers, who were used to having runtime changes of settings be reflected in the cfg later 'automatically', get frustrated if that doesn't happen anymore and write broken buggy code that doesn't check for null (even if opening the cfg file in the game dir never works) or writes to a non-portable path to do the 'same' now.

That's how the engine previously behaved right? A setting like language changed at runtime in-memory representation would be written out to a cfg file on exit? And no longer does now because the file will not be written automatically ever?

@rofl0r
Copy link
Contributor

rofl0r commented Apr 17, 2021

developers, who were used to having runtime changes of settings be reflected in the cfg later 'automatically', get frustrated if that doesn't happen anymore and write broken buggy code

that's why i'd suggest the engine provides a script function like SaveConfigToFile() that game authors can call explicitly, rather than having to rely on the behaviour of the engine.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Apr 17, 2021

that's why i'd suggest the engine provides a script function like SaveConfigToFile() that game authors can call explicitly, rather than having to rely on the behaviour of the engine.

I've been considering this, but my concern is how to restrict what does this function save, so that it won't save something that was not expected again, such as windowed/fullscreen for example (which may have changed simply by a key press).

Another concern is that developer may want to save something that is not currently active in game. For example, if they want to ask user to restart the game for the changes to take place.

There are other variants, such as providing interface to reading and saving config options by key/value too. But these would require additional api for convenient work and avoid any problems with backwards compatibility. With so little options that may be changed at runtime at the moment this sounds like too much, so perhaps this idea may be left for future consideration.

Well, I can of course add a simple function to "save every config option that is now active". Which maybe will work for starters for people who do not want to program anything.

@ivan-mogilko
Copy link
Contributor

Hm, okay, for a simple solution we could have this "SaveConfig" with an argument accepting a set of flags. Because currently there are like 3 or 4 options that may be changed at runtime, and int32 gives us 32 flag slots, that may be enough for a time being, leaving more sophisticated API ideas for ags4.

Also, I'd probably need to add some kind of $USERCONFIGFILE$ token for File.Open, to let it open user config wherever it is. Because there will be developers who like to use IniFile script module to read or write options there (it could be some already do this).

@rofl0r
Copy link
Contributor

rofl0r commented Apr 17, 2021

hmm, before overdesigning this, maybe we should ask some developers in AGS forums about their opinion ?

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Apr 18, 2021

I may ask of course. It's just that there's no guarantee that anyone will answer. Maybe two people will reply, and others won't, or will in a few months, or a year...

I may also make a "save config" function without arguments, because if arguments has to be added later they may be declared with default value so won't break scripts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context: user config res: in consideration type: enhancement a suggestion or necessity to have something improved
Projects
None yet
Development

No branches or pull requests

7 participants