-
Notifications
You must be signed in to change notification settings - Fork 64
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
[MIRROR] Fix a few vendor restocking bugs #2910
Merged
Merged
Conversation
This file contains 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
* Fix a few vendor restocking bugs (#82603) ## About The Pull Request So while working on fixing standard RPEDs working at all, I noticed they weren't displaying parts when used on a vendor. Looking into it, it seemed to be because the vendors just called `display_parts(user)` on its own: https://github.com/tgstation/tgstation/blob/1780fcef62e0413da65375a37f7c8030a0371419/code/modules/vending/_vending.dm#L1151 While other machinery would wrap it in a `to_chat(...)` call. https://github.com/tgstation/tgstation/blob/1780fcef62e0413da65375a37f7c8030a0371419/code/game/machinery/_machinery.dm#L971 So, we do that too! But then, during further testing, I noticed it wasn't actually restocking. Ever. Even though it's calling the `restock(...)` proc with the same inputs as you would do manually- oh. _oh._ https://github.com/tgstation/tgstation/blob/1780fcef62e0413da65375a37f7c8030a0371419/code/modules/vending/_vending.dm#L1152-L1154 It's checking the replacer for whether it's a refill canister, and not the contents of it. Has been doing this for about 8 months, apparently. No wonder it wasn't working. Great! This should work, right? One more round of testing! Aaaaand it doesn't actually pay out any credits when restocking this way. That's because it's *specifically* coded on `attackby(...)`. https://github.com/tgstation/tgstation/blob/1780fcef62e0413da65375a37f7c8030a0371419/code/modules/vending/_vending.dm#L707-L715 Alright, well, we move this to a new `post_restock(...)` proc, and call this whenever we're done doing restocks. ```dm /obj/machinery/vending/proc/post_restock(mob/living/user, restocked) if(!restocked) to_chat(user, span_warning("There's nothing to restock!")) return to_chat(user, span_notice("You loaded [restocked] items in [src][credits_contained > 0 ? ", and are rewarded [credits_contained] credits." : "."]")) var/datum/bank_account/cargo_account = SSeconomy.get_dep_account(ACCOUNT_CAR) cargo_account.adjust_money(round(credits_contained * 0.5), "Vending: Restock") var/obj/item/holochip/payday = new(src, credits_contained) try_put_in_hand(payday, user) ``` This is separate from the `restock(...)`, so we can call it when we're _done_ restocking and need a message, rather than sending one for each canister in our RPED. We can't just break the loop on the first fitting canister, because the items we need might be spread over multiple partially full canisters. Third round of testing anyone? Oh hey infinite money glitch. It seems like we never actually reset the `credits_contained` var after paying out with it. Soooo we just append this to our `post_restock(...)` proc, and be done with it. ```dm /obj/machinery/vending/proc/post_restock(mob/living/user, restocked) (...) try_put_in_hand(payday, user) credits_contained = 0 ``` At this point, I felt it better we also reorganized the vendor `exchange_parts(...)` to only have one `display_parts(...)` call and an early return if we can't exchange parts. Any further issues I could find I felt were outside of the scope, and better off atomized into a separate pr. ## Why It's Good For The Game Fixes RPEDs not working with vendors at all. Fixes RPEDs not displaying vendor parts. Fixes restocking vendors with an RPED not giving credits. Fixes vendors not resetting their contained credits when restocked. ## Changelog :cl: fix: RPEDs can be used on vendors again. Note that only bluespace RPEDs can carry vendor refills as of writing. fix: RPEDs can display vendor parts again. fix: Restocking vendors gives credits whether done manually or by RPEDs. fix: Vendors reset their contained credits when restocked. /:cl: --------- Co-authored-by: san7890 <[email protected]> * Fix a few vendor restocking bugs --------- Co-authored-by: _0Steven <[email protected]> Co-authored-by: san7890 <[email protected]>
AnywayFarus
added a commit
that referenced
this pull request
Apr 16, 2024
Iajret
pushed a commit
that referenced
this pull request
Jun 8, 2024
…m to use Templates for ID cards [MDB IGNORE] (#2910) * Plexagon Access Management now tells you that you need a Trim to use Templates for ID cards (#83683) ## About The Pull Request Plexagon Access Management now removes the Templates dropdown, and puts in a note about trim in its place, if the inserted card does not have a trim attached. ## Why It's Good For The Game You can't apply a template without a trim on the card. The proc that handles it does a bitwise AND between what the template and the trim have for access, and applies it to the ID card. This is not mentioned anywhere in the program. I spent hours tracing ID procs trying to find the supposed bug preventing templates from applying before I stumbled upon this. So now it tells the user. I do not know why you're allowed to choose templates that aren't related to the currently applied trim, but that's out of scope for my frustration. ## Changelog :cl: qol: Plexagon Access Management now tells you that you need an ID Trim before applying a Template, rather than silently failing. /:cl: * Plexagon Access Management now tells you that you need a Trim to use Templates for ID cards --------- Co-authored-by: zxaber <[email protected]> Co-authored-by: NovaBot13 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Mirrored on Nova: NovaSector/NovaSector#2008
Original PR: tgstation/tgstation#82603
About The Pull Request
So while working on fixing standard RPEDs working at all, I noticed they weren't displaying parts when used on a vendor.
Looking into it, it seemed to be because the vendors just called
display_parts(user)
on its own:https://github.com/tgstation/tgstation/blob/1780fcef62e0413da65375a37f7c8030a0371419/code/modules/vending/_vending.dm#L1151
While other machinery would wrap it in a
to_chat(...)
call.https://github.com/tgstation/tgstation/blob/1780fcef62e0413da65375a37f7c8030a0371419/code/game/machinery/_machinery.dm#L971
So, we do that too!
But then, during further testing, I noticed it wasn't actually restocking. Ever. Even though it's calling the
restock(...)
proc with the same inputs as you would do manually- oh. oh.https://github.com/tgstation/tgstation/blob/1780fcef62e0413da65375a37f7c8030a0371419/code/modules/vending/_vending.dm#L1152-L1154
It's checking the replacer for whether it's a refill canister, and not the contents of it. Has been doing this for about 8 months, apparently. No wonder it wasn't working.
Great! This should work, right? One more round of testing!
Aaaaand it doesn't actually pay out any credits when restocking this way.
That's because it's specifically coded on
attackby(...)
.https://github.com/tgstation/tgstation/blob/1780fcef62e0413da65375a37f7c8030a0371419/code/modules/vending/_vending.dm#L707-L715
Alright, well, we move this to a new
post_restock(...)
proc, and call this whenever we're done doing restocks.This is separate from the
restock(...)
, so we can call it when we're done restocking and need a message, rather than sending one for each canister in our RPED. We can't just break the loop on the first fitting canister, because the items we need might be spread over multiple partially full canisters.Third round of testing anyone? Oh hey infinite money glitch.
It seems like we never actually reset the
credits_contained
var after paying out with it.Soooo we just append this to our
post_restock(...)
proc, and be done with it.At this point, I felt it better we also reorganized the vendor
exchange_parts(...)
to only have onedisplay_parts(...)
call and an early return if we can't exchange parts.Any further issues I could find I felt were outside of the scope, and better off atomized into a separate pr.
Why It's Good For The Game
Fixes RPEDs not working with vendors at all.
Fixes RPEDs not displaying vendor parts.
Fixes restocking vendors with an RPED not giving credits.
Fixes vendors not resetting their contained credits when restocked.
Changelog
🆑 00-Steven
fix: RPEDs can be used on vendors again. Note that only bluespace RPEDs can carry vendor refills as of writing.
fix: RPEDs can display vendor parts again.
fix: Restocking vendors gives credits whether done manually or by RPEDs.
fix: Vendors reset their contained credits when restocked.
/:cl: