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
Money achievements #42198
Money achievements #42198
Conversation
This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there: https://discourse.cataclysmdda.org/t/your-ideas-for-new-achievements-and-conducts/24008/35 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions. I can see arguments against these suggestions, if there are no other uses for my proposed generalizations, but the events you've added seem over-specific to me.
if( u.cash < 100000000 && ( u.cash + amount ) >= 100000000 ) { | ||
g->events().send<event_type::becomes_millionaire>( u.getID() ); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The events you've added are very niche. I feel like this one should e.g. be a generic 'deposit' event, which includes the amount deposited and the new balance. I think deposits are rare enough that shouldn't overwhelm the log.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you suggest here is future-proofing, even though I can't imagine any specific use for it offhandedly, so I'll do it.
if( cc->ammo_remaining() == cc->ammo_capacity( money ) ) { | ||
g->events().send<event_type::fills_cashcard>( u.getID() ); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly this could be a 'transfer_money' event with the new balance.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered. |
This issue has been automatically closed due to lack of activity. This does not mean that we do not value the issue. Feel free to request that it be re-opened if you are going to actively work on it |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered. |
This issue has been automatically closed due to lack of activity. This does not mean that we do not value the issue. Feel free to request that it be re-opened if you are going to actively work on it |
Summary
SUMMARY: Content "Money achievements"
Purpose of change
To add more achievements
Describe the solution
Added two new, money related achievements:
Describe alternatives you've considered
N/A
Testing
Spawning 1000 cash cards in game and building a house of cards.
Additional context
N/A