B1n-ry / Youre-in-grave-danger

A minecraft mod which adds one single block. The grave, which will generate when you die, and store your items
https://www.curseforge.com/minecraft/mc-mods/youre-in-grave-danger
MIT License
59 stars 16 forks source link

Campanion Backpacks #132

Open SaphireLattice opened 9 months ago

SaphireLattice commented 9 months ago

Describe the bug The Campanion backpacks spill out on death rather than get saved. They are supposed to spill their contents when you pull them off, so I guess it defaults to that.

To Reproduce Steps to reproduce the behavior:

  1. Go to the nether while wearing any of Campanion backpacks
  2. Die
  3. Realize with horror while on death screen that your backpack has just spilled all over, likely into lava

Expected behavior Backpack does not spill items out and is restored perfectly fine when you get to the grave

Desktop (please complete the following information):

Additional context Using Campanion mod (v4.1.2) on a modified friend Tinkerer's Quilt modpack

SaphireLattice commented 9 months ago

Tried to implement this myself. For some reason I am getting an error about "private method" for BlockEntityRendererFactories, but shrug. Maybe messed up dev env setup.

As I understand it, there's no way to merge existing inventory with the one from the grave? I am confused as to how dropAll method works, as it runs the same way for both picking up and dying, as I understood? But when dying that method has to make sure that everything got cleared so nothing drops the usual way, no? But if I do the same when picking it up, that wipes out the alive player inventory... I'm rather confused.

Unless for case of "prevent mod from dropping items from special slot" I should be doing the housekeeping in the getInventory method when onDeath is true?

SaphireLattice commented 9 months ago

Nevermind, I messed up lol

getInventory I had did not actually set anything up when it was called with onDeath = false. Which I did not realize handled merging of two inventories (alive and dead) by just using that on both dead and alive inv.

Anyways, for anyone looking to implement compat, here's rough guideline for stuff:

Also, is there any good way to merge two ItemStack together? Did not have the best success looking for that

B1n-ry commented 9 months ago

Um right... Yeah I can see some stuff might not be the easiest to understand.. But yes I think you basically got how inventory saving and grave stuff works. I'm trying to write an easier way (or at least not as confusing way) to add compat with inventory mods in my 1.20 rewrite, which is taking some time. As with item stacks being "mergable", I don't think that was something that I ever considered would be doable. I'll make sure to try and do something like that for the 1.20 rewrite though. If there's still any uncertainty, don't hesitate to ask

SaphireLattice commented 9 months ago

TBH it isn't that bad of an API. Just confusingly named and could really use some typing and docs help. The former is a bit finickly to get all done. For docs it's mostly "what these methods need to do" and such. As despite the comments on the interface, I couldn't really get what these do without extensive looking at both other compat APIs and when and how mod calls this all.

For mergeable - I just sorta made my own lil' hack where it does forEach on maybe extra items list and tried to see if it can combine that into an existing stack in the mod inventory.

Also, is there any reason not to let extra items from soulbound handler to go into player inventory/drop as overflow? Cuz the mod in question has a backpack that by itself does not store anything and it's all just in the player entity. And I wanted so soulbound stuff in the backpack gets pulled through. But turns out the YIGD ignores the extra from that. So I have to leave it in the grave instead, as otherwise it will just get wiped. Or I guess I could make it get dropped but that sounds... off.

Edit: Checked the 1.20. Wow that got changed a bunch huh. But pretty nice, and feels more readable! And nicely unifies handling on the backend for soulbound/respawn stuff and for grave pickup. Though, still ignores extra items from the respawn handler...

B1n-ry commented 9 months ago

I'm not gonna lie, I completely overlooked that part, and didn't realize I didn't do that already. Thanks for pointing it out. Might not change in the 1.19 branch since I've kinda moved on from working on that version, but could perhaps go back there in the future. Will fix in 1.20 branch though

SaphireLattice commented 9 months ago

By the way, could I make a PR for 1.19 branch to get this in? If it looks sufficiently okay, that is

Though, urgh, another jar to add isn't exactly pretty. Makes the repo bloat in size...

B1n-ry commented 9 months ago

If not another jar, use cursemaven or something. That's what I'm doing on the 1.20 branch anyway