diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.04k stars 791 forks source link

Proposed Stash Functionality #1049

Closed joewis closed 2 years ago

joewis commented 3 years ago

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.~~

2) 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.

3) 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.

FitzRoyX commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago
  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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

https://github.com/diasurgical/devilutionX/pull/1054

AJenbo commented 3 years ago

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 commented 3 years ago

#1069 Adria / Griswold buyback functionality for item 3

karaluh commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.

xGnoSiSx commented 2 years ago

Hi guys! I'm looking at this and at https://github.com/diasurgical/devilutionX/issues/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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Most of it is already implemented. Just needs a weekend of tying things together: https://github.com/diasurgical/devilutionX/pull/3853

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

xGnoSiSx commented 2 years ago

perfect.

AJenbo commented 2 years ago

Closing this as the basic are now merged. There are still a few outstanding issues which are tracked here: https://github.com/diasurgical/devilutionX/issues/4211