Previously we used None as the sentinel for determining that a user had
not specified a default value for a field. This meant that a user could
never select `None` as the default value. We now properly use UNSET to
determine if a default was passed, this aligns better with the code as
it stands given that if no default is passed we'll use UNSET anyway.
* Check for Unset type in Emoji.custom
* Typecase id value to bool
in the case where id is Unset this will call the builtin
__bool__ which returns False. This is not ideal beacuse
an Id of 0 will mean the emoji is custom, but to fix would
require a deeper refactor of the Field logic. Lets put a pin
in it.
* Remove unused import
Previously to update a role you would update attributes of the role
object, and then call save. This was very much an anti-pattern when
compared to the way things work elsewhere, and had the following
annoyances attached to it;
- Updating attributes of the object actually smashes state, we aren't
guarenteed that the user will call save nor that the API will accept our
save, and thus attributes on the object (which could be shared within
the state module) are smashed and innaccurate.
- Guild.update_role is effectively useless in this case, we can only
ever pass a role object in. This makes partial updates impossible, and
forces us to match the OO interface to use it.
The new style follows along with how we do things elsewhere, there is
still likely some additional work that can be done here to allow passing
in the role, but for now we'll just call this a deprecation
This commit is a fairly large chunk of code that fixes a previously
annoying and nasty bug causing cached_properties to not be cleared.
Alongside this bug fix I took the opportunity to refactor the entire
methdology behind cached properties, and bind them more strictly to the
behavior of models.
Prior to this commit, cached_properties where not be properly reset upon
a model's `update` method being called due to some invalid code. Along
with this issue, the actual behavior of cached properties landed in a
weird in-between/gray area where they had to support both non-model and
model use cases.
The new version of cached_property is strictly for use with models, and
another version `simple_cached_property` was added to support other use
cases. The new cached_property simply builds an actual `property` within
the metaclass. This is fairly efficient, and also reduces the surface
area of behavior.
When writing this I messed around with a few ideas, including allowing
the user of `cached_property` to define a linkage between the property
and fields that should invalidate its cached value, but this was both
messy and introduced massive cognitive load on understand when a
cached_property should be cleared. Although this version is slighty less
efficient, I'm very much in favor of it vs the alternatives I tried.
Previously guild_id sat outside of the object which created a
semi-invalid role object without guild information. This would be saved
in the guild state.
One option for fixing this would just to be fixing the guild_id of the
role within the state, however I think it's responible to expect roles
passed from the event to have valid context/guild attributes.
Kodie Goodwin
andrei
Andrei Zbikowski
bunnyhero
Alex
ryan
Ondřej Slabý