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

ID card computer bug fixed (And made it more fun!) #32308

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

beck-thompson
Copy link
Contributor

@beck-thompson beck-thompson commented Sep 19, 2024

About the PR

Bug reported on discord by Kaz!

When deconstructing the ID card computer, the inserted IDs would get deleted. I added the same component that machines use so this should fix any other bugs with computers deleting stuff (Although I think there aren't any currently?).

I also made the ID computer specifically throw the IDs out in a fun way instead of just dropping them. It also now ejects the IDs when damaged! See the video in the comments.

Media

2024-09-19.10-25-07.mp4

Changelog

🆑 Beck Thompson, Kaz Wooblie

  • fix: Computers now wont delete items inside of them when deconstructing.
  • add: The ID card computer now throws the IDs out when deconstructed or damaged!

@dvir001
Copy link
Contributor

dvir001 commented Sep 20, 2024

What about on destroy?

@lzk228
Copy link
Contributor

lzk228 commented Sep 20, 2024

  • fix: Computers now wont delete items inside of them when deconstructing.

  • add: The ID card computer now throws the IDs out when deconstructed!

i think the second line in cl really means the first, so you left only the first one

@beck-thompson
Copy link
Contributor Author

i think the second line in cl really means the first, so you left only the first one

They are similar but not quite the same! This PR could technically just be a one line YMAL change (Adding the MachineDeconstructedEvent) but the default behavior is to just spawn the items right on top of the computer which isn't fun (But it does work). Adding that fixes the bug where it gets deleted!

The second part is the unique logic for only the ID computer (Other computers won't do this) which I make it spit out the ids randomly!

@beck-thompson
Copy link
Contributor Author

What about on destroy?

I'm really busy right now but if isn't merged soon I'll add it! Sounds fun

@beck-thompson
Copy link
Contributor Author

2024-09-21.00-15-36.mp4

@0x6273
Copy link
Contributor

0x6273 commented Sep 21, 2024

Why is eject on damage needed? I'm pretty sure you can get the IDs out if you right click, even if the computer is broken.

@beck-thompson
Copy link
Contributor Author

Why is eject on damage needed? I'm pretty sure you can get the IDs out if you right click, even if the computer is broken.

Its fun

@github-actions github-actions bot added the Merge Conflict This PR currently has conflicts that need to be addressed. label Sep 23, 2024
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the Merge Conflict This PR currently has conflicts that need to be addressed. label Sep 24, 2024
@slarticodefast
Copy link
Member

slarticodefast commented Sep 24, 2024

Its fun

Adjust your PR title and description for this please.
Also you just turned a quick bugfix PR into one in need of a three day maintainer dicussion due to gameplay changes 😄

@beck-thompson beck-thompson changed the title ID card computer bug fixed ID card computer bug fixed (And made it more fun!) Sep 24, 2024
@beck-thompson
Copy link
Contributor Author

Also you just turned a quick bugfix PR into one in need of a three day maintainer dicussion due to gameplay changes 😄

Good I like giving more work to the maintainers 😈 😈 😈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants