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

Design for game-specific customization and mods #108

Open
slipher opened this Issue May 14, 2018 · 11 comments

Comments

Projects
4 participants
@slipher
Copy link
Contributor

slipher commented May 14, 2018

We have not yet decided how game-specific customization for the engine is going to work. One attempt was #16, but people found some problems with it. It is important to realize that there are some game-specific things in the engine that are more complicated than just replacing a string, such as:

  • List of teams for team-specific key binds (1) (2)
  • Differing meaning of surface flags in maps: #107

One question is: Should one be able to run any Daemon game using the same Daemon binary, or should game details be baked into the engine's executable? Some people think that it is desirable to be able to use the same Daemon executable, but personally I don't see any tangible benefit from this. To share the same engine binary between installations of multiple games seems to be an obviously bad idea for many reasons (ping me if you disagree), so I am assuming that each game installation will have its own copy of daemon. Given that, game-independence has no benefit to the user or even to developers that I can see. Nevertheless, I am not opposed to loading configuration at runtime if it actually works the best from a technical point of view.

The obvious alternative that I see would be to have a configuration header file defined by each game, which is included into the engine when building it. This header would contain the definitions like #define PRODUCT_NAME "Xonotic" or const char* const bindTeams[] = {"spectators", "humans", "aliens"};. There could be a default definitions file for building daemon by itself for CI or whatever. Not a particularly pretty solution, but neither is having to write a parser for various types of constants.

Mods

It would be good idea to think about the design for mods too, since we might be able to reuse some configuration mechanisms between games and mods within a game. Some of the issues a mod system needs to address:

  • Ensure that files downloaded by mods can never affect the main game.
  • Decide which parts of the user's configuration (key bindings, cvars...) should have per-mod instances, versus be shared among all mods. In the case of key bindings, a mod may need its own configuration if it defines many new commands. On the other hand, if a mod uses exactly the same controls but mod-specific configuration is used (assumed to be generated on the first play by copying the base game's bindings), it would be annoying that a stale copy of the keybindings is used every time after the first time it is played.
  • Be robust against mod developer cluelessness. Imagine a mod system where a mod identifies itself by simply declaring its name. And then mod "foo" is allowed to write packages in the "mods/foo" directory. But you know that some guy who wants to create a mod completely unrelated to the foo mod is going to start out by copying the foo source tree, modifying a lot, but neglecting to ever change the name of the mod. Then if the user visits servers hosting both kinds of "foo" mod, both mods are broken afterwards.
@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented May 14, 2018

Things can be done part by part. For example the questions after Mod heading are very interesting but can be designed later. For the things described before Mod heading, there is some things we already know there is no need to rebuild the engine for, and for them a simple config file in libpath would be good and even if we don't know yet how to do the other things (surfaceflags, keybindings…) I don't see any reason to not do what we can do.

Do the remaining bits that need some code can be a shared library? Or is it stupid? This not precisely to enable player to run multiple games with the same binary, but to make the engine more modular. Being able to run multiple games with the same binary would just be an unsupported side effect (that can be a feature on very-well managed environments like a distro though, but as a nice re-purposable feature).

@slipher

This comment has been minimized.

Copy link
Contributor Author

slipher commented May 14, 2018

@illwieckz I would prefer to decide at least the game configuration mechanism sooner rather than later. I don't want us to spend a bunch of time modifying things to work with one method, but then decide it's not flexible enough.

I'm not seeing what the advantages of the shared library approach over the header file would be (you can even write functions in the header if necessary). The disadvantages of the shared library would be:

  • Another separate binary means chances for errors due to version mismatches between the library and daemon (this kind of stuff is a real drag on developer productivity).
  • We have to rewrite all the parts that depend on configuration being known at compile time.
@MarioSMB

This comment has been minimized.

Copy link

MarioSMB commented May 14, 2018

For what it's worth, that team bind code definitely belongs in Unvanquished, not Daemon.
DarkPlaces has launch parameters (or uses the executable name) to get the game's profile: https://gitlab.com/xonotic/darkplaces/blob/div0-stable/common.c#L1448

The other approach of defining it in a config or separate file seems like a nice way to not require hardcoding games at all though, and it may be preferred to an ever-growing built in limited list.

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented May 14, 2018

@MarioSMB I edited your comment to fix the link :-)

@DefaultUser

This comment has been minimized.

Copy link
Contributor

DefaultUser commented May 14, 2018

In Darkplaces you can define multiple bindmaps and then use one as foreground and another (or the same) as background/fallback for keys that are not in the foreground map. I think that this idea could be adapted:

  • mods define their own keymap overlay
  • games can define surfaceflags or teams (I think teams should be defined by the game, not the engine)

However, some game-specific variables (like those that I proposed to extract in #16) should not fall back to defaults and maybe even shutdown the engine

@illwieckz illwieckz added this to To Do in Smokin' Guns Oct 24, 2018

@illwieckz illwieckz added this to To Do in Xonotic Oct 24, 2018

@illwieckz illwieckz added this to To do in UnrealArena Oct 24, 2018

@illwieckz illwieckz added this to To do in Unvanquished Oct 24, 2018

@illwieckz illwieckz added this to To do in Standalone via automation Oct 24, 2018

@illwieckz illwieckz removed this from To do in Unvanquished Oct 24, 2018

@illwieckz illwieckz removed this from To do in UnrealArena Oct 24, 2018

@illwieckz illwieckz removed this from To do in Xonotic Oct 24, 2018

@illwieckz illwieckz removed this from To do in Smokin' Guns Oct 24, 2018

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Mar 4, 2019

I think I just found the optimal way to handle per game keybindings by looking for a solution to an issue we already have for modding.

The issue we have is that team are stored as numbers in the keybinding configuration file, this number being the index of a table like { "default", "aliens", "humans", "spectators" }:

static const char *const teamName[] = { "default", "aliens", "humans", "spectators" };

The issue is that once a mod adds a third race, this third race would inherit the spectators keybinding, which is bad. It would have been possible to fix this early by storing the spectators team after the default one and before the playable ones. But it was not done this way and it's now too late.

We also have to face the fact the only way to change team orders in that array while automatically porting user binds is to change the storing format so the engine can both detect the bind was not yet ported and the bind was already successfully ported. If we just swap numbers, we cannot detect if the conversion was already done or not.

This comes to this idea.

The game send a cvar to the engine containing the custom teams. I don't know yet if spectator has to be part of builtin engine team or custom game team, but since the need for spectator is very broad, let's say we will do it this way.

So the engine has this builtin array:

{ "default", "spectators" }

And the game send this string in a cvar: aliens humans, so the engine extends the array this way:

{ "default", "spectators", "aliens", "humans" }

The magic is to also use these team names to store the keybind in file, so we have to implement a porting code that read this:

bind hw:0 +command0
teambind 1 hw:1 +command1
teambind 2 hw:2 +command2
teambind 3 hw:3 +command3

and port it this way:

bind hw:0 +command0
teambind aliens hw:1 +command1
teambind humans hw:2 +command2
teambind spectators hw:3 +command3

The engine can still handle internally the bind as indexes since numbers are fast, but the idea would be that the game send the teambind command to the engine with a string, and the engine stores this string as is in the keybinding.cfg file and computes the index from the cvar for in game usage. While reading the keybinding.cfg, the engine would also read the team name and compute the index from a cvar.

The awesomeness of this is that the team index would never have to go outside of a game and would become a pure internal and ephemeral value. Even if the given cvar is filled with humans aliens instead of aliens humans on the next game it would work.

@DefaultUser

This comment has been minimized.

Copy link
Contributor

DefaultUser commented Mar 4, 2019

The problem I see with teambinds is that the user cannot define their own bindmaps if the game/mod does not define another team. So maybe the engine can implement a general bindmaps feature that the games can then use to implement teambinds or modspecific binds if needed.

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Mar 4, 2019

There could be a default definitions file for building daemon by itself for CI or whatever. Not a particularly pretty solution, but neither is having to write a parser for various types of constants.

I don't think there is a need for more than key value pairs, so we can abuse the existing config parser, basically the engine would load a daemon.cfg file that would be stored in libpath right to the daemon binary, and that file would contains something like that:

b_gameName    Unvanquished
b_master1     master.unvanquished.net
b_master2     master2.unvanquished.net
b_wwwBaseURL  dl.unvanquished.net
b_URIScheme   unv

Those would be the built-in from which other strings are computed (like home path) or cvar set (cl_gamename, sv_master1, sv_wwwBaseURL…) if unset. We have to make sure the engine cannot read other config files from this directory and we have to make sure those cvar are read only.

The good thing is that since IRC support was nuked, there is less things to take care about. It's better if the pkg string is standard and unique for the Dæmon engine. It means third-party tools like map editors or map compilers only have to know the homepath/userpath and it enables them to automatically compute path while having an option to work from asset source repository while relying on the other standard src prefix. This way that pkg string is not something to discriminate games but something to enable a workflow. Since Dæmon would be thought for easy shipping standalone games from the start (people would not have to rebuild the engine), full conversion games would never have to rely on this kind of trick to live in another game home directory anymore, they would be able to live on their own home directory with a stock engine.

@illwieckz

This comment has been minimized.

Copy link
Member

illwieckz commented Mar 4, 2019

The problem I see with teambinds is that the user cannot define their own bindmaps if the game/mod does not define another team. So maybe the engine can implement a general bindmaps feature that the games can then use to implement teambinds or modspecific binds if needed.

Is this a call for overlay bind?

Why not something like that:

overlay ctf bind hw:0 +command0
overlay ctf teambind aliens hw:1 +command1
overlay ctf teambind humans hw:2 +command2
overlay ctf teambind spectators hw:3 +command3

If overlay keyword, then read the overlay profile name then the command as if there was no overlay. Once an overlay is loaded, like with loadOverlay ctf, those commands would be run.

Note that it would also allow for specific options like graphical optimizations.

So people would be able to do this:

bind F5 loadOverlay ctf
overlay ctf bind hw:1 +activate vaporizer
overlay ctf bind hw:2 +activate blaster
overlay ctf set r_normalMapping 0
overlay ctf set r_parallaxMapping 0

But I think this is outside the scope of that issue. It's more a feature request.

@DefaultUser

This comment has been minimized.

Copy link
Contributor

DefaultUser commented Mar 5, 2019

I really like the idea of an "overlay" keyword. Integrating that into a menu might be a bit harder, but I don't think that this is a valid reason against such a system.
However, if every mod can define its own overlays like that, we need to implement it properly to prevent mods from modifying core settings like the masterserver cvars.
overlay evil bind hw:1 'set masterserver evil.xyz' and overlay evil set masterserver evil.xyz should either be prohibited or only set that cvar while that mod is loaded. But then we also need to store that in the configs somehow. Especially that first one might be hard to catch.

@slipher

This comment has been minimized.

Copy link
Contributor Author

slipher commented Mar 6, 2019

I agree this is a decent solution for team binds - essentially allowing binds to have a tag and the cgame to load or unload binds associated with the tag. Could also be interesting for mods that add extra commands.

I don't see that overlay bindings pose additional security issues beyond the ones that already exist. But yeah, at some point I plan to make a whitelist of which binds the cgame can create, as well as restrict which cvars it can set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.