ValveSoftware / halflife

Half-Life 1 engine based games
Other
3.66k stars 616 forks source link

[Feature request] Add command "differences" to show all convars that have a differente value than default. #2346

Open metita opened 5 years ago

metita commented 5 years ago

This is already integrated in source games.

This will show all convars which are not at their default values, so it might be useful to modify convars more easily if they are modified.

SamVanheer commented 5 years ago

Cvars don't track what their default value is so this is impossible.

metita commented 5 years ago

Cvars don't track what their default value is so this is impossible.

Even if cvars have a default value set by the game itself? if cvar differs from the default value then display it on console via differences.

SamVanheer commented 5 years ago

Default values are hardcoded wherever they're reset, there is no way to track the values in one location without hardcoding it in the handler for the command.

For example in R_ForceCVars:

if ( r_lightmap.value != 0 )
{
  Cvar_DirectSet(&r_lightmap, "0");
}
metita commented 5 years ago

Maybe that should be a improvement too, change the way cvars are being treated on the code itself.

SamVanheer commented 5 years ago

That would break compatibility with mods.

MOCOLONI commented 5 years ago

Cvars don't track what their default value is so this is impossible.

Source games have no problem with that. I'm sure the feature can be ported into GoldSrc.

2010kohtep commented 5 years ago

The solution I see is to create a new field in 'cvar_t' - 'char *first' at the end of the structure that would keep the value of 'string' of the registered cvar, which was passed to 'Cvar_RegisterVariable'. This would not break mods compatibility, I think, and would allow to implement what we want.

Sent with GitHawk

afwn90cj93201nixr2e1re commented 5 years ago

https://github.com/ValveSoftware/halflife/issues/2312

SamVanheer commented 5 years ago

Cvars don't track what their default value is so this is impossible.

Source games have no problem with that. I'm sure the feature can be ported into GoldSrc.

Source didn't have to maintain binary compatibility with member variables. Even then they had to implement workarounds to deal with mods using different versions of the SDK to access newer features in the cvar code: https://github.com/ValveSoftware/source-sdk-2013/blob/0d8dceea4310fde5706b3ce1c70609d72a38efdf/mp/src/public/tier1/convar.h#L287-L293

This was only possible because in Source cvars are classes with virtual functions to handle this stuff. In GoldSource cvars are structs that are entirely handled by the engine.

The solution I see is to create a new field in 'cvar_t' - 'char *first' at the end of the structure that would keep the value of 'string' of the registered cvar, which was passed to 'Cvar_RegisterVariable'. This would not break mods compatibility, I think, and would allow to implement what we want.

Sent with GitHawk

And how will the engine know if the cvar has that field? You'd need a way to signal the engine, like a flag or using a new function to register it.

Even then the engine has to keep track of this meaning it would have to allocate memory to create the cvar list to store the extra data in, in addition to maintaining the linked list so anything that uses that won't break. Any mods that grab the memory address of the cvar list will then crash upon trying to use it because it will no longer be a linked list of cvar_t instances.

Using flags is dangerous because mods may have re-purposed unused flags. If any commands that use this new variable were to be executed the engine would think it exists and access memory that might not contain anything, resulting in garbage data or crashes.

I don't see any way to do this that won't at the very least risk mods crashing.

2010kohtep commented 5 years ago

Right. I forgot that cvar pointer in is passed Cvar_RegisterVariable, which is saved to the list, and it is not copied into separate memory. In this case, it is very difficult to implement such functional, and it is better to abandon this idea.

afwn90cj93201nixr2e1re commented 5 years ago

В чем собственно проблема? Ведь у нас есть поинтер и мы можем сделать shadow копию до его изменения и сохранить его в его же member после его изменения, после этого удалить неиспользуемый shadow.

SamVanheer commented 5 years ago

@shelru please speak English so we can all understand.

I ran what you said through translate and if what you're suggesting is to create copies of cvar_t then it won't work. A lot of code access the cvar instance directly so all you'll be doing is making a cvar that isn't referenced by code.

Even if you keep a pointer to the original it will still change the engine's code for cvars enough to break anything that relies on hooking internal functions, and the next pointer will be harder to use, if it would even have one.

afwn90cj93201nixr2e1re commented 5 years ago

@SamVanheer please speak Russian, so we can all understand. But seriously, that message related to kohtep, this is the reason why it on Russian.

So, imagine. some cvar's array/vector etc. every element has a pointer. before changing we create shadow copy like: bool abc(cvar_t ptr, value){ cvar_t shadow; some memcopy/memset shadown with value from ptr->value; ex.: shadow->value = ptr->value; ptr->value = value; ptr->shadow = shadow; delete shadow; }

I can't get where is the problem? In size? Padding when value > old_value? Or what?

SamVanheer commented 5 years ago

What happens when the cvar instance doesn't have the shadow variable?

JoelTroch commented 5 years ago

@shelru GitHub is a worldwide place and therefore the main language of communication is english.

You are not helping everyone by speaking russian. If you want to speak russian, do it in private and/or on a russian dedicated community.

afwn90cj93201nixr2e1re commented 5 years ago

As i said message was addressed to koh., but i get u.

And what should happen? nothing, no shadow, meaning the value didn't change, shadow by default is nullptr

SamVanheer commented 5 years ago

Mods would register cvars that don't have shadow so this code will access memory that could be another object, or not allocated at all, which will likely crash.

afwn90cj93201nixr2e1re commented 5 years ago

I can't get what r u talking about. Cvar register is engine func? We can implement global cvar register for engine, and get cvar pointer/size for mods. Idk what's wrong with that implement way.

afwn90cj93201nixr2e1re commented 5 years ago

Why mods should have access to shadow? Shadow is only for shadow copies. Engine manage shadows, ex. at disconnect and restore values.

SamVanheer commented 5 years ago

Mods that use the current cvar_t structure won't have shadow variables in their cvars, so when the engine tries to access it it won't be a valid memory location. This is a common issue with changing the layout of classes and interfaces.

afwn90cj93201nixr2e1re commented 5 years ago

Also mods must be rebuilt at the same time as the engine and it is obvious.

SamVanheer commented 5 years ago

Many mods' source code has been lost and most of the rest will never see another release. Backwards compatibility is more important than one nice to have feature.

afwn90cj93201nixr2e1re commented 5 years ago

Here some official mods which still support and have pages on steam store. I think valve have access to src if they r posted it in store.

Anothers mod seems unofficial. Here no support for non-steam store version's.

It's not a problem to replace some sdk and fix mods.

SamVanheer commented 5 years ago

Plenty of mods are made for the Steam version of Half-Life and will break if the changes you've proposed were to be made.

It is in fact a serious problem if such a thing were to be done, which is why it won't happen.