This repository was archived by the owner on May 22, 2025. It is now read-only.
Achievement Get Part 3 - Cargo Achievements (and bugfixes!) #7917
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Altoids1
added
the
WORK IN PROGRESS
Contributor
|
Assistant |
Altoids1
changed the title
Altoids1
changed the title
The use of ckey() is actually enough to ensure an inability to do SQL injection.
Altoids1
removed
the
WORK IN PROGRESS
Contributor
Author
Ready for review. |
Hopekz
approved these changes
Mar 25, 2020
alexkar598
suggested changes
Mar 25, 2020
code/modules/economy/account.dm
Outdated
| if(M.real_name == account_holder) | ||
| SSachievements.unlock_achievement(/datum/achievement/cargo/bourgeois, M.client) | ||
| is_bourgeois = TRUE | ||
| break |
Member
There was a problem hiding this comment.
You can't assume there aren't going to be any duplicated names due to genetic fuckery or something like that
Suggested change
| break |
Comment on lines
49
to
51
| message_admins("[ADMIN_LOOKUPFLW(usr)] has launched an artillery strike.") | ||
| if(usr.client) | ||
| SSachievements.unlock_achievement(/datum/achievement/cargo/bsa, usr.client) |
Member
There was a problem hiding this comment.
This is an admin only version, not the cargo not
Suggested change
| message_admins("[ADMIN_LOOKUPFLW(usr)] has launched an artillery strike.") | |
| if(usr.client) | |
| SSachievements.unlock_achievement(/datum/achievement/cargo/bsa, usr.client) | |
| message_admins("[ADMIN_LOOKUPFLW(usr)] has launched an artillery strike.") |
| for(var/x in GLOB.player_list) | ||
| var/mob/M = x | ||
| if(M.real_name == account_holder) | ||
| SSachievements.unlock_achievement(/datum/achievement/cargo/bourgeois, M.client) |
Member
There was a problem hiding this comment.
Please add a check within the unlock_achievement proc to check if the client exists, its too easy to forget a if(whatever.client)
yogstation13-bot
added a commit
that referenced
this pull request
Mar 28, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds four more achievements, as part of #7657, of the Cargonian variety.
Coder Warning
For some reason, #7751 made both read and write of the database entries for achievements no longer work.
I have... no real idea why, but I've decided to revert Alexkar's changes and do the sanitation through new methods.
All achievements now use the
initial()variants of their properties to prevent tampering.In addition, all ckeys used in queries are run through
ckey(), which would remove any and all special characters that're necessary to do a SQL injection.Doing it this way should not only be more performative (using native BYOND procs instead of several sequential calls to bsql.dll) but also actually work.
Changelog
🆑 Altoids
rscadd: Cargo now has a couple achievements of their own!
bugfix: Fixed bug that caused achievements to not load nor save.
/:cl: