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

Simplify Usage of Settings #203

Open
Frag1337 opened this issue Jun 8, 2017 · 4 comments
Open

Simplify Usage of Settings #203

Frag1337 opened this issue Jun 8, 2017 · 4 comments

Comments

@Frag1337
Copy link
Contributor

Frag1337 commented Jun 8, 2017

Hey,

I was trying to use the Player.settings module and thought, if it would be easier to use it than via the menu.

How about if we just simply could use:

Player.settings.get_string_setting('test_bool')

or

Player.settings.save_string_setting('test_bool', True)

This would allow us much more ways to modify player settings, such as in commands, in a all in one settings menu or anything else.

@satoon101
Copy link
Member

I guess I am not fully understanding the context as to what you are requesting. I don't think modifying a player's settings should really happen in a plugin, it should be up to the player themselves to modify their own settings.

Also, in case you are unaware, outside of using the SP player settings menu on each server, users can also add things like the following to cfg files on their client:

setinfo some_player_setting 1

For instance, in my MostDamage plugin, I add PlayerSettings with the md prefix, and then add a Location string setting:
https://github.com/satoon101/MostDamage/blob/master/addons/source-python/plugins/most_damage/most_damage.py#L37-L38
https://github.com/satoon101/MostDamage/blob/master/addons/source-python/plugins/most_damage/most_damage.py#L85-L90

So now, clients can now use the following in any of their cfg files:

setinfo md_location 1

That value is always checked first. If it is found that the client does not have that convar set locally, we then grab the value from the server. So, if a client goes onto a lot of servers that have MostDamage installed, and they always want to have the same setting, they can simply add that to their autoexec.cfg file in their client files.

I realize the documentation is lacking, and that is something I have planned to work on but just haven't. I also definitely need to add in functionality for users to dump all SP player settings on a server, so that they have access to all of these client convars. Though, the plugins that utilize this functionality themselves should also add them as a part of their documentation.

If I have failed miserably at this response, please just clarify what you are intending to want added (and why) with further context.

@Frag1337
Copy link
Contributor Author

Frag1337 commented Jun 9, 2017

I tried to give it a chance, next to my 'clientprefs' version from sourcemod to source.python. (Which simply let the plugin easily save the players choice for the named setting).

So, I was usually having my own chat commands, or my own settings menu (Toggle, with custom ON/OFF Outputs, sometimes even more than that with cycling).

After building it up, theres only few ways to set it, with that built in menu (for one setting?) or that console command. It would be pretty nice to just use a my own command like !hudoff which disables 3 player settings at once, or making my own toggle commands. Its hard to explain, but If its still not understandable Im trying it another way.

Thanks

@satoon101
Copy link
Member

Well, again, player settings aren't really meant to be set by plugins themselves. Typically, the user should do as I stated above, and if they want to toggle their settings, they could run a client command to execute the appropriate cfg file. However, if you wish to circumvent this, you could always use something like the following:

from players.entity import Player
from settings.player import PlayerSettings
from settings.storage import _player_settings_storage

plugin_player_settings = PlayerSettings('My Plugin Name', 'prefix')
setting_one = plugin_player_settings.add_bool_setting(
    name='Setting One',
    default=True,
)
setting_two = plugin_player_settings.add_bool_setting(
    name='Setting Two',
    default=True,
)
setting_three = plugin_player_settings.add_bool_setting(
    name='Setting Three',
    default=True,
)

def some_function(user_index):
    player = Player(user_index)
    for setting in (setting_one, setting_two, setting_three):
        _player_settings_storage[player.uniqueid][setting.convar] = not setting.get_setting(user_index)

@KirillMysnik
Copy link
Member

I remember talking with Frag on this matter. As for me, the key thing is:
So, I was usually having my own chat commands, or my own settings menu (Toggle, with custom ON/OFF Outputs, sometimes even more than that with cycling).

If I understoood Frag correctly, he builds up a custom settings menu JUST because of how inconvenient current implementation is. Indeed, to toggle a boolean setting, you have to click on the corresponding setting, and that opens another submenu. While Frag's own implementation just switches (toggles) the setting upon "clicking" on it, without entering a submenu. For a bunch of boolean settings that really simplifies the usage, IMO.

Then to extend this further, Frag wanted to be able to cycle through possible setting values the same way as with boolean "toggling". E.g. click - value1, another click - value2, another click - value3, another click - value1 again.

I think that this can be implemented as a keyword (like open_submenu, default is True) in the add_bool/int/string_setting etc.

I'd also throw my two cents in here. I think that some sort of EnumSetting would be more pythonic than the current StringSetting. Really, you can't set StringSetting to arbitrary values via chat nor via convar anyways (and from its name, I'd say that you could), so it's bound to a set of values. Why not just bind it to an enum.Enum then? I believe that comparing a setting this way:

if quake_sound_volume.get_setting(player.index) == "loud":
    ...

is less pythonic than

if quake_sound_volume.get_setting(player.index) == SoundVolume.LOUD:
    ...

Actual text shown in the menu for each value can be either set as a SoundVolume value:

class SoundVolume(Enum):
    NORMAL = strings['sound_volume normal']
    LOUD = strings['sound_volume loud']

But in this case I don't see the benefits of the Enum class, i.e. a regular class can be used;

Or it can be supplied some other way, say, via a separate mapping:

plugin_settings.add_enum_setting(
    name="quake_sound_volume",
    enum=SoundVolume,
    default=SoundVolume.LOUD,
    text=plugin_strings['settings volume'],
    value_translations={
        SoundVolume.LOUD: plugin_strings['settings value loud'],
        SoundVolume.NORMAL: plugin_settings['settings value normal']
    }
)

This allows to use an IntEnum as a base class with all its benefits. To simplify this, value_translations can be set to None, then the name for each enum value might be obtained this way:

# `value` is SoundVolume.LOUD
value.name.title()  # 'Loud'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants