Achievements contrib #3447
Achievements contrib #3447
Conversation
InspectorCaracal
force-pushed
the
achievements-contrib
branch
from
c2750c3
to
be9ad02
Compare
There was a problem hiding this comment.
It's a cool idea! I think this can do with some improvements and clarifications though.
I also have a few use-cases I'm unsure about, such as how to 'just' add an achievement to a user based on an external condition (something which I've found to be the most common case whenever I've done achievements).
InspectorCaracal
force-pushed
the
achievements-contrib
branch
from
9db3e0d
to
137ebaa
Compare
InspectorCaracal
force-pushed
the
achievements-contrib
branch
from
137ebaa
to
d8a0ebc
Compare
There was a problem hiding this comment.
Improvements for sure 👍 , I'm not sure I see some of my comments as actually being addressed though.
As a point of PR etiquette, please don't "resolve" review comments on your own. It just makes it harder for the reviewer - since there are several commits making changes harder to track what changed, and it's not clear if a resolved comment means it's closed because you disagreed with the comment or if you agreed and implemented it. Easiest is if you reply to the comment and say if you implemented the change, or why you disagree and won't (like you did in one place). Leave the "resolving" of review comments to the reviewer. :)
You also did a bunch of other changes to this PR after the last review, without commenting on what these changes were. This makes it hard to know what should be re-reviewed from scratch and what were just refactoring without changing functionality.
|
|
||
| ### Defining your achievements | ||
|
|
||
| This achievement system is designed to use ordinary dicts for the achievement data - however, there are certain keys which, if present in the dict, define how the achievement is progressed or completed. |
There was a problem hiding this comment.
After changes, I still don't see anything about where these dicts are defined.
| #### Examples | ||
|
|
||
| A simple achievement which you can get just for logging in the first time. This achievement has no prerequisites and it only needs to be fulfilled once to complete, so it doesn't need to define most of the keys. | ||
| ```python |
|
|
||
| In this case, `obj` is the fruit that was just purchased, and `quantity` is the amount they bought. | ||
|
|
||
| The `track_achievements` function does also return a value: an iterable of keys for any achievements which were newly completed by that update. You can ignore this value, or you can use it to e.g. send a message to the player with their latest achievements. |
There was a problem hiding this comment.
Not sure my question was answered?
|
|
||
| def at_use(self, user, **kwargs): | ||
| # track this use for any achievements about using an object named our name | ||
| finished_achievements = track_achievements(user, category="use", tracking=self.key) |
There was a problem hiding this comment.
This question is still open to me.
|
@Griatch Ack! Sorry about that. I will refrain from marking things resolved in the future. 🫡 This is the only project I contribute to and I'm not very good with the github interface. |
|
@InspectorCaracal No worries, these things are not obvious in the GH interface and won't come to mind until after one has reviewed a few |
|
|
||
| ### Defining your achievements | ||
|
|
||
| This achievement system is designed to use ordinary dicts for the achievement data - however, there are certain keys which, if present in the dict, define how the achievement is progressed or completed. |
There was a problem hiding this comment.
Yes, but it's not mentioned here, where the actual dicts are described, which is confusing to the reader. A line 'These dicts are defined in Python modules you tell Evennia to import" or something would clarify things.
| ``` | ||
|
|
||
|
|
||
| ## Installation |
There was a problem hiding this comment.
I note you moved this down. I can see some reason for this in that the previous sections are introducing the system. But all other contribs put the installation as close to the top as possible, so it's probably better to follow that.
|
|
||
| In this case, `obj` is the fruit that was just purchased, and `quantity` is the amount they bought. | ||
|
|
||
| The `track_achievements` function does also return a value: an iterable of keys for any achievements which were newly completed by that update. You can ignore this value, or you can use it to e.g. send a message to the player with their latest achievements. |
|
@InspectorCaracal I had missed to answer one of your follow-up questions in the code comments, did so now :) |
|
@InspectorCaracal This is almost done, with just some minor doc comments to fix :) |
|
I did a major revision of the README and I think it covers all the issues now. |
|
Oops, was just doing another pass to make sure I didn't miss anything and realized I left some tab indents, so I'll be putting in another commit in a second just to fix those. |
Brief overview of PR changes/additions
Adds a new contrib which provides a mechanism for tracking and checking game achievements.