Daemon icon indicating copy to clipboard operation
Daemon copied to clipboard

Feature request: make cgame cvars readable by name without IPC

Open illwieckz opened this issue 1 year ago • 3 comments

The idea is to implement a special Cvar feature to be used in cgame.

One may declares a cvar this way:

Cvar::Cvar<bool> cg_drawFPS("cg_drawFPS", "show client's frames per second", Cvar::NONE, true);

Then later in some initialization code, one would do:

Cvar::MySide(cg_drawFPS);

This syntax is similar to the Cvar::Latch feature implemented in engine, allowing someone to mark a cvar as latched one.

The Cvar::MySide function would be used by cgame developers to instruct the Cvar system that this Cvar is declared on the same side of the code calling the Cvar::MySide function, here the cgame.

This function would buil-up a map of cvar names and cvar pointers (here { "cg_drawFPS", &cg_drawFPS }) to be used later when a cvar is checked by name string (like in RmlUI code) doing something like this:

std::string value = Cvar::GetValue( "cg_drawFPS" );

When doing that, the Cvar::GetValue would first look into the map. If the cvar is registered as being on same side, Cvar::GetValue would just read the cvar value from the local object without doing any IPC. If the cvar was not registered as being on same side, it would then fallback on the IPC.

illwieckz avatar May 06 '24 23:05 illwieckz

To summarize a discussion by @illwieckz and I, we think this can be done with the following actions:

  • Add a batch set cvar message
  • Modify the Proxy Cvar::GetValue to read the CvarMap directly. If a cvar doesn't exist in cvarmap, we register it and cache the value.
  • Modify the Proxy Cvar::SetValue track changed cvars and write to the cvar map.
  • At the end of the frame, we read changed cvars and call the batch set cvar message. Then we clear the list.

DolceTriade avatar May 07 '24 04:05 DolceTriade

  • Modify the Proxy Cvar::GetValue to read the CvarMap directly. If a cvar doesn't exist in cvarmap, we register it and cache the value.

To observe cvars registered by the sgame or engine from the cgame, we will need the ability to register a cvar in multiple modules. This is not currently possible, but I plan to implement it as soon as I finish migrating engine cvars to new-style cvars. (Status: migration script mostly ported to work on the engine, now I just need to actually run it).

As for asynchronous cvar setting, I hope this would be an opt-in API (e.g. Cvar::SetValueAsync) and not something mandatory for all cvars. It would be annoying to have to worry about this all the time. As an example of something where you'd have to worry about that, consider the custom admin command feature. This executes a script file from config/ with arguments passed as cvars. It would probably still work as trap_SendConsoleCommand is also buffered and executed later, but it's something you have to think about and could break if the command API were different.

As for the OP idea of observing cvars registered within the same module, this would be doable without too much effort (and without an engine release). The cgame is already subscribed to updates of its own cvars and stores their values. Two possible implementations:

  1. Add a GetSerializedValue() function to CvarProxy that returns the cvar's value as a string. Look up a cvar in the cgame's cvar map and use GetSerializedValue to get it as a string. The thing that's a little wonky with this approach is that the stringified value may be different than the string stored on the engine side. (And this may even result in a different numerical value after reparsing as I don't think our float serializer actually round trips...)
  2. Store the string version of all cvars in the CvarMap. This would use more memory, but the advantage is that it would be 100% correct, so we could actually use it as an authoritative answer in the Cvar::GetValue implementation.

slipher avatar May 07 '24 18:05 slipher

I wasn't sure whether to make this opt-in or opt-out. Depending on the number of callsites, (ie, if a lot, make it opt-out, if only few, its ok to make it opt-in).

I think your proposal for storing the stringified value is good and we should do that.

DolceTriade avatar May 07 '24 21:05 DolceTriade

I would probably leave this open until we get the multiproxy support in too because non-cgame cvars used in the UI will still trigger IPC.

DolceTriade avatar May 13 '24 01:05 DolceTriade