Skip to content

Player.from_userid caching #297

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

Merged
merged 3 commits into from
Jan 9, 2020
Merged

Conversation

satoon101
Copy link
Member

  • Added caching boolean argument for Player.from_userid.

@satoon101 satoon101 self-assigned this Jan 4, 2020
@satoon101
Copy link
Member Author

This came about because I was testing GunGame this morning and encountered an issue with gg_multi_level. I narrowed down the issue to the fact that the cache was causing me to get the same instance of _MultiLevelPlayer every time. There were other ways to fix it and use the cache, but I at least thought it would be a good idea to allow for caching=False through from_userid.

@jordanbriere
Copy link
Contributor

jordanbriere commented Jan 4, 2020

Yes, makes sense for that method to forward that keyword. We should also add it to Entity.from_inthandle as well. Since it breaks compatibility, I believe we should default it to False here instead of True so that existing subclasses that were not implemented with caching in mind continues to works as they used to.

EDIT : I suggest to change that section to:

        # Set whether or not this class is caching its instances by default
        try:
            cls._caching = signature(
                vars(cls)['__init__']
            ).parameters['caching'].default
        except KeyError:
            cls._caching = vars(cls).get('caching', False)

To get the following behaviours:

class MyEntity(Entity):
    pass

print(MyEntity.caching) # False


class MyEntity(Entity):
    caching = True

print(MyEntity.caching) # True


class MyEntity(Entity):
    def __init__(self, index, caching=True):
        pass

print(MyEntity.caching) # True

Also, from_userid/from_inthandle should ideally default caching to None so that it internally follows cls.caching for subclasses.

…tly enable it (to retain backward compatibility for existing classes that were not implemented with caching in mind).

Added caching keyword to Entity.from_inthandle.
Fixed Player.from_userid not following the caching state of the current class unless explicitly specified.
Added missing documentation.
@jordanbriere jordanbriere merged commit 4c97b94 into master Jan 9, 2020
@jordanbriere jordanbriere deleted the bugfix_player_from_userid branch January 9, 2020 04:22
jordanbriere added a commit that referenced this pull request Apr 9, 2020
…r instances by default (introduced by the backward compatibility fix added into #297).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants