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

Add artifact bag of holding #13964

Merged
merged 43 commits into from Nov 6, 2023
Merged

Conversation

FlameArrow57
Copy link
Contributor

@FlameArrow57 FlameArrow57 commented May 4, 2023

[GAME OBJECTS][FEATURE]

About the PR

This PR adds a new handheld artifact, the bag of holding.

The bag of holding is an artifact that acts as a storage, just like a box, but that has more variety in how it stores things or acts.

It comes in three varieties, depending on origin.

Eldritch-
Eldritch bags of holding are a general variety storage, which can hold anywhere from a few to a good amount of items, from tiny to bulky. Higher size capacities are more rare for higher slots. However, this type has no visible HUD, and items stored in them are stored in a FIFO, LIFO, or random order. FIFO/LIFO/random order idea from Flourish.

Martian-
Randomized, "shape-shifting" storage. Every so often, its storage slots and max holdable size will randomize. They can always be worn on your belt or back.

Wizard-
This is a large storage artifact that can hold from a few to a lot of items from tiny to small sized. However, you can only see a smaller number of the items in the storage at a time, and every time you add an item, take an item out, or look in the storage, you see a random set of items. Idea from zjdtmkhzt.

Larger storages can be worn on belts, for Martian, or your back, for all types. Artifacts transform their icon state when put in a belt or back slot.

Faults can trigger when storing items.

If you manage to put a bag of holding into a bag of holding, something bad happens too which can include
-Explosions
-Black holes
-Getting trapped in a pocket dimension
-Contents teleporting randomly

A side effect of the current rustle effect with how handheld artifacts are coded/sprited right now is that the sprite will flip sideways or upside down for rustles sometimes

Sprites made by DimWhat (Wizard sprites by my self)-
image
image

Why's this needed?

It'd be cool to see a handheld/wearable storage artifact, since that would allow for more fun and varying storage items

Changelog

(u)FlameArrow57 and DimWhat
(*)Added the bag of holding artifact! Appears with three variants. Ones with larger storages can be worn on your back, and the Martian one on your belt too.

@keywordlabeler keywordlabeler bot added A-Game-Objects The point of this PR is to deal with a specific game object C-Feature A new feature or enhancements to existing features labels May 4, 2023
@github-actions github-actions bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 4, 2023
@flrsh
Copy link
Contributor

flrsh commented May 4, 2023

I definitely like the idea of storage artifacts, though storage in general can be rather annoying to balance and I'm unsure about just making these straight up Better Storage. Personally I'd like these more if there were weirder aspects you had to work around, e.g. FIFO / LIFO, all of one type share the same storage, etc.

@FlameArrow57
Copy link
Contributor Author

FlameArrow57 commented May 4, 2023

Okay, yeah, makes sense. FIFO/LIFO/random pick is possible through another PR I'm working, #13961, which I think would be fun to add, I'll try to work on some more things like that

@zjdtmkhzt
Copy link
Contributor

This seems really cool!

Yeah, storage is a hard thing to balance, in addition to things like LIFO/FIFO/random there could also be something like a type with infinite storage, but which only ever shows a random selection from all stored items as available.

Maybe watch out with the naming on things like "fault" for the burning stuff, because artifact faults are already a specific artifact mechanics, so it could potentially be confusing to talk about in coder talk.

If some of these can be worn, maybe there should be a worn sprite for them? I was under the impressions most larger wearable containers, like backpacks or belts had them.

@amylizzle
Copy link
Contributor

Very cool, though personally I think all artifacts ought to be activated before they can be used. I've been meaning to fix artifact beakers not doing that for a while now.

@Exester509
Copy link

For the wizard one, it would be really cool if on fail it swapped the item for a note saying "This item has been confiscated by the Space Wizard Federation" or something similar. I'd find it a hilarious fail

@Glamurio
Copy link
Contributor

Some of these should have a chance to spawn with a random array of items in them, akin to how beakers have random chems. Would add an additional layer of lootbox fun to finding one, rather than just for storage-sake.

@github-actions github-actions bot added the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label May 26, 2023
@FlameArrow57
Copy link
Contributor Author

FlameArrow57 commented May 27, 2023

For the wizard one, it would be really cool if on fail it swapped the item for a note saying "This item has been confiscated by the Space Wizard Federation" or something similar. I'd find it a hilarious fail

That'd be fun, though idk how much Wizards are tied to the artifacts in lore, and might change up the fail effects

@FlameArrow57
Copy link
Contributor Author

FlameArrow57 commented May 27, 2023

Some of these should have a chance to spawn with a random array of items in them, akin to how beakers have random chems. Would add an additional layer of lootbox fun to finding one, rather than just for storage-sake.

Agree that'd be cool, was thinking that's a thing for the container artifacts, was trying to not do the same effect

@github-actions github-actions bot removed the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label May 28, 2023
@boring-cyborg boring-cyborg bot added the A-Mapping A mapping change label May 29, 2023
@github-actions github-actions bot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 16, 2023
@FlameArrow57
Copy link
Contributor Author

FlameArrow57 commented Sep 16, 2023

Okay, added Martian storage, everything should work to what I tested, ready for re-review

Things from when this was last worked on

  • Might just do away with the Precursor storage
  • Not sure if there's too much in terms of the combining bags of holding effects
  • Code is finished
  • Sprites are still WIP

@github-actions github-actions bot added S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict and removed S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict labels Sep 17, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2023

This PR has been inactive for two weeks, and has been automatically marked as stale. This means it is at risk of being auto closed in another week. Please address any outstanding review items and ensure your PR is finished. If you are auto-staled anyway, ask developers if your PR will be merged. Once you have done any of the previous actions then you should request a developer remove the stale label on your PR, to reset the stale timer. If you feel no developer will respond in that time, you may wish to close this PR youself, while you seek developer comment, as you will then be able to reopen the PR yourself.

@github-actions github-actions bot added the S-Stale An inactive PR that has had no updates in the past two weeks label Oct 9, 2023
@github-actions github-actions bot removed the S-Merge-Conflict Applied and removed when a PR has or no longer has a merge conflict label Oct 26, 2023
@FlameArrow57
Copy link
Contributor Author

Ready for review again

@ZeWaka ZeWaka removed the S-Stale An inactive PR that has had no updates in the past two weeks label Nov 5, 2023
@Tarmunora Tarmunora merged commit 1e88ac5 into goonstation:master Nov 6, 2023
25 checks passed
github-actions bot pushed a commit that referenced this pull request Nov 6, 2023
@FlameArrow57 FlameArrow57 deleted the artifact-storage branch November 7, 2023 01:59
@Studenterhue Studenterhue added the E-Add-To-Wiki A PR that will require changes to the wiki label Nov 12, 2023
@ZeWaka ZeWaka removed the E-Add-To-Wiki A PR that will require changes to the wiki label Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Game-Objects The point of this PR is to deal with a specific game object A-Mapping A mapping change C-Feature A new feature or enhancements to existing features C-Sprites Automatically applied on any .dmi or icons folder change size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants