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

Proposed Stash Functionality #1049

Closed
joewis opened this issue Feb 24, 2021 · 24 comments
Closed

Proposed Stash Functionality #1049

joewis opened this issue Feb 24, 2021 · 24 comments
Labels
enhancement New feature or request
Milestone

Comments

@joewis
Copy link
Contributor

joewis commented Feb 24, 2021

Heroes notoriously run out of space and players regularly have to make the frustrating decision to sell items in order to make space for new loot. This is in conflict with the the concept of a loot based game, which tends to attract hunter/gatherers, collectors, hoarders and traders. Such a feature is present in D2 and D3 and feels missing in D1.

1) Private Stash
Allow the hero to store items / gold at Ogen's Tavern of the Rising Sun, implying the hero rented a room there (Maybe against a charge, to make gold more useful. Also Ogden only has one customer, who drinks for free). A private stash may also allow for a further enhancement, an armory feature similar to D3.

  1. Shared Stash
    Add a storage functionality to Farnham, to rent out storage space (So he can pay for his drinks). Since Farnham is not the most reliable in keeping track of which hero stored what, other locally saved heroes with access to the same storage space will be able to take any hero's stored items. Currently this is can be achieved in a less convenient fashion with "mule" characters who carry items between multiplayer games.

  2. Temporary Stash
    Not exactly a stash, but allow to buy back items from Griswold and Adria. This would allow to store items for the duration of the game, and prevent to lose items when accidentally selling them.

@joewis joewis closed this as completed Feb 24, 2021
@joewis joewis reopened this Feb 24, 2021
@FitzRoyX
Copy link

FitzRoyX commented Feb 24, 2021

My opinions on stash: 1) I don't think having a private stash is necessary. Your stash should just be accessible by all local saves. 2) I think Ogden should have an "Access Stash" menu entry and a "Show/Hide Stash" menu entry. The stash is hidden by default so that the map doesn't look altered with a chest on it unless you want it to. The benefits of showing the stash is that it's one less click to access.

I don't think that the stash should cost money as built-in QoL. A modder could go farther and do that though.

@galaxyhaxz
Copy link
Member

  1. I don't think having a private stash is necessary. Your stash should just be accessible by all local saves.

I agree, you're the only person I've seen mention this. I think whenever you click on "Access Stash" at Odgen, it should list all heroes on that computer and you click that character to view their stash and take/add items to it. This would prevent the need of having a unique save file type for just the stash.

@NiteKat
Copy link
Contributor

NiteKat commented Feb 24, 2021

I think some players like an isolated stash for solo self-found type stuff, but even then, if there's both an isolated stash per character + a shared one that all characters can access, one could easily transfer stuff from the shared to the isolated one...

@joewis
Copy link
Contributor Author

joewis commented Feb 25, 2021

My thought was to have one file stash.sv, and probably a stash.hsv to be shared between all local characters. This would be item 2) Shared Stash and requires the most effort, and retains compatibility.

To avoid having a stash file, it might be possible to save the stash in the player save files and loop through all local save files.
I may be mistaken, but the with the same approach as for saving hotkeys this should retain compatibility. Private stash could be implemented first, and later enhanced to a shared stash. This would also take care of separating single from multi and sv from hsv. It also leaves Farnham free for other functionality - I really want to get him a job.

Regarding gold or not - nothing in Tristram is free, except for healing, and that's just Pepin honoring the Hippocratic oath. But granted, it would be annoying (like Wirt's location or the 5K max gold).

@FitzRoyX
Copy link

  1. I don't think having a private stash is necessary. Your stash should just be accessible by all local saves.

I agree, you're the only person I've seen mention this. I think whenever you click on "Access Stash" at Odgen, it should list all heroes on that computer and you click that character to view their stash and take/add items to it. This would prevent the need of having a unique save file type for just the stash.

That still sounds like a degree of character-specific space, and it adds another click. I'm wondering why the stash shouldn't just be a common inventory associated with no particular character? Yes, that would require it being its own save file. Is that bad?

@galaxyhaxz
Copy link
Member

No, it's not, but I think a private stash is unnecessary if each save has its own stash and you may access others.

@joewis
Copy link
Contributor Author

joewis commented Feb 25, 2021

No, it's not, but I think a private stash is unnecessary if each save has its own stash and you may access others.

Private stash would be a first step, actually the store functionality comes first. I think this from my perspective - what I can possibly achieve at my current skill level, so small steps are key.
I'd happily agree with any version of shared stash if someone wants to implement it :)

@FitzRoyX
Copy link

I'm also assuming that the stash would be at least 100 pages deep, so if you wanted to dedicate certain pages to certain characters, you could.

@malvarenga123
Copy link

IMO, one important thing to think before implementing it is the actual format of the stash file. It should be extensible enough so that it works seamlessly with Hellfire and mods. Most likely something in the direction of #614

@qndel
Copy link
Member

qndel commented Feb 25, 2021

#1054

@AJenbo AJenbo added the enhancement New feature or request label Feb 25, 2021
@AJenbo
Copy link
Member

AJenbo commented Feb 26, 2021

imo the stash should be shared in multiplayer, but not single player. This way it doesn't change what is possible to do compared to a normal game.

@joewis
Copy link
Contributor Author

joewis commented Feb 28, 2021

#1069 Adria / Griswold buyback functionality for item 3

@AJenbo AJenbo added this to the 1.3.0 milestone Mar 20, 2021
@karaluh
Copy link

karaluh commented May 20, 2021

imo the stash should be shared in multiplayer, but not single player. This way it doesn't change what is possible to do compared to a normal game.

You can transfer items between characters in single player in Hellfire.

@agris-codes
Copy link

agris-codes commented Jun 29, 2021

imo the stash should be shared in multiplayer, but not single player. This way it doesn't change what is possible to do compared to a normal game.

Very much disagree, sharing a stash in a singleplayer game amongst all the locally saved players would be very welcome.

BUT, your point about "doesn't change what is possible to do compared to a normal game" is partially addressed by Hellfire and cornerstone of the world. That being said, this functionality (stash for single player & multi player) probably makes sense in a distinct branch of devilutionX, like a community edition, containing additional QOL features that also change what is possible to do as compared to a normal game.

Core devilutionX probably isn't the place for stash functionality. Games with extensive modding support sometimes have fan patches that are split between straight fixes, and fixes + common community requested QOL features and is termed a "Community Edition". Here, I'm thinking that concept applies to the source-remake of Diablo and devilutionX in particular.

Maybe I'll start a discussion on this topic but I think (1) a singleplayer stash is highly desirable (2) adding a stash isn't appropriate for "base" devilutionX, and (3) stash access through Gillian makes the most sense.

edit: clarified intent

@qndel
Copy link
Member

qndel commented Jun 29, 2021

Actually who needs a stash in singleplayer when you can just dump items in town 🙄 I don't think we are going to maintain 100 branches with different features, if someone cooks a good stash, it gets into master and that's it, it's supposed to be one of main 1.3.0 features and a lot of people are waiting for it

@agris-codes
Copy link

Actually who needs a stash in singleplayer when you can just dump items in town 🙄 I don't think we are going to maintain 100 branches with different features, if someone cooks a good stash, it gets into master and that's it, it's supposed to be one of main 1.3.0 features and a lot of people are waiting for it

You should read the thread, we're talking a stash shared amongst all locally saved single player characters, i.e. not what you're addressing.

@AJenbo AJenbo modified the milestones: 1.3.0, 1.4.0 Sep 12, 2021
@xGnoSiSx
Copy link
Contributor

Hi guys!
I'm looking at this and at #614
To speed things up and not take any difficult routes, perhaps do the JSON format switch first, and perhaps implement stashes without the tetris inventory system and instead re-use the vendor list, without dealing with inventory x,y w,h positioning.

That would be a great first step for 1.4 - most people would even be ok, with just copy pasting some JSON between character saves or some external stash txt file - this game is way past any anti cheat reasoning at this stage and we all play for fun. If anyone wishes to implement closed realm functionality, I wish them infinite luck, because the source doesn't take any steps to protect against memory manipulation anyway. MPQ and encryption on player data serves no purpose other than to make life more difficult.

As for the format and backwards compatibility, have the legacy saves on another GUI option or have a configuration variable in the ini to fall back to the old binary style and another one for converting saves.

For sure, other devs would feel more inclined to develop something like GDStash, if only the JSON feature is just there. But most right now just look for a way to pass items between characters.

And we/you can discuss and decide the final direction on 1.5.
Regards!

@julealgon
Copy link
Contributor

To speed things up and not take any difficult routes, perhaps do the JSON format switch first

I think the conversion from MPQ to JSON is probably more complex than the stash itself, but I might be wrong.

and perhaps implement stashes without the tetris inventory system and instead re-use the vendor list, without dealing with inventory x,y w,h positioning.

The tetris inventory part is actually fairly trivial, considering it is already done for the normal inventory and can easily be reused. I wouldn't worry too much about that aspect of the implementation in regards to effort.

@xGnoSiSx
Copy link
Contributor

xGnoSiSx commented Jan 17, 2022

To speed things up and not take any difficult routes, perhaps do the JSON format switch first

I think the conversion from MPQ to JSON is probably more complex than the stash itself, but I might be wrong.

and perhaps implement stashes without the tetris inventory system and instead re-use the vendor list, without dealing with inventory x,y w,h positioning.

The tetris inventory part is actually fairly trivial, considering it is already done for the normal inventory and can easily be reused. I wouldn't worry too much about that aspect of the implementation in regards to effort.

How so? The source is already minimalist as it is. If the basic json just re-uses player and item field names then that's just it.

As it is, there's no real support anywhere on adding/modding items and their properties. if someone mods the source to add stuff he has to mod the JSON part as well, and we either lock the modded saves, or discard the parts that are invalid.

@StephenCWills
Copy link
Member

How so? The source is already minimalist as it is. If the basic json just re-uses player and item field names then that's just it.

As it is, there's no real support anywhere on adding/modding items and their properties. if someone mods the source to add stuff he has to mod the JSON part as well, and we either lock the modded saves, or discard the parts that are invalid.

The original format compresses the data by saving the RNG seed for item recreation. If you only focus on the effort involved in saving and loading JSON, then indeed it wouldn't be so hard to just throw out the old format and break compatibility. However, keep in mind that both Cornerstone and the network messages still use the compressed item format so you can't just throw out the old save format and expect to be able to freely edit item properties.

I'd also expect that supporting both formats simultaneously would represent a significant amount of technical debt, mostly when it comes to assessing issue reports. I'm not the one who'd be making this call, but unless we get a really solid PR, I suspect that we won't be ready to make the leap to JSON until we're ready to break bidirectional compatibility with the original format.

@xGnoSiSx
Copy link
Contributor

xGnoSiSx commented Jan 18, 2022

Let's not break all the code contracts at once then, but storing the seed in the JSON as well, is still simple. Now on changing values on items, I suppose you can't do it without the proper seed, and that's ok.

Just trying to keep it simple here, even if the items are stored as just the seed in JSON with a label for the name is cool too! I can already see how the codebase is piling design choices on top one another in very simple code that is screaming to be expanded x10.

Doing custom moddable items with custom modded properties is a major undertaking. this and the JSON link are not the proposals for it.

@AJenbo
Copy link
Member

AJenbo commented Jan 18, 2022

Most of it is already implemented. Just needs a weekend of tying things together: #3853

Regarding copy pasting content in json, i think the vast majority of players will find this unreasonably difficult.

@xGnoSiSx
Copy link
Contributor

perfect.

@AJenbo
Copy link
Member

AJenbo commented Mar 21, 2022

Closing this as the basic are now merged. There are still a few outstanding issues which are tracked here: #4211

@AJenbo AJenbo closed this as completed Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests