Skip to content
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] Indestructible items are properly rejected by material containers #2008

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

Steals-The-PRs
Copy link
Collaborator

Mirrored on Nova: NovaSector/NovaSector#993
Original PR: tgstation/tgstation#81487

About The Pull Request

  • Fixes #81486

Happens only with items that are Indestructible like the pai card, It's a 2 part fix

  • First display error message saying the item was rejected
  • If the item is indestructible but was put inside another object like an bag or anything else, then move the object to turf where this object was inserted(context) & not the ore silo(parent) thus fixing the teleportation bug

Changelog

🆑 SyncIt21
fix: Indestructible items like the pai card don't teleport to the ore silo when you insert them into silo linked machine & also displays a message saying it was rejected.
/:cl:

…iners (#993)

* Indestructible items are properly rejected by material containers (#81487)

## About The Pull Request
- Fixes #81486

Happens only with items that are Indestructible like the pai card, It's
a 2 part fix
- First display error message saying the item was rejected
- If the item is indestructible but was put inside another object like
an bag or anything else, then move the object to turf where this object
was inserted(`context`) & not the ore silo(`parent`) thus fixing the
teleportation bug

## Changelog
:cl:
fix: Indestructible items like the pai card don't teleport to the ore
silo when you insert them into silo linked machine & also displays a
message saying it was rejected.
/:cl:

* Indestructible items are properly rejected by material containers

---------

Co-authored-by: SyncIt21 <[email protected]>
@Iajret Iajret merged commit 06f03e5 into master Feb 17, 2024
24 checks passed
@Iajret Iajret deleted the upstream-mirror-993 branch February 17, 2024 16:21
AnywayFarus added a commit that referenced this pull request Feb 17, 2024
Iajret pushed a commit that referenced this pull request Apr 16, 2024
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants