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

Compatibility with Traveler's Backpack #124

Closed Tiviacz1337 closed 5 months ago

Tiviacz1337 commented 1 year ago

Hello, I'd like to add compatibility with You're in Grave Danger from my end, if it's possible of course and if it's better solution. I see you've implemented compat from your end and I'd like to ask first. Is it better to add compat from my side or yours?

B1n-ry commented 1 year ago

I assume the question is to move the compatibility from my end to your end? As I have compatibility already provided with a few other mods that can extend the player inventory in some regards, I feel like it's probably easiest for that kind of compat on my end. Although since it already exists, is there anything you would like to change in the compat implementation? (Just curious why you asked if there already is a working compatibility between the two mods). Is there something you feel like it's not working, or you want some extra compat features, you can let me know. If you have some arguments to why the compat should be on your end, it'd be great for me to know about any potential upsides to that.

Tiviacz1337 commented 1 year ago

So basically, I have no problem with compat on your end, I'm just curious if everything is done properly. I mean, I just added compat for few gravestone mods on Forge. And since TB has feature that places backpack after death, I was able to cancel those methods if any gravestone mod was detected. Don't know if it happens with your compat implementation though.

Also is compat added on every version (1.16.5, 1.17.1, 1.18.2, 1.19.2, 1.19.4 and 1.20.1)?

B1n-ry commented 1 year ago

Well yigd deletes all items from the player that it stores in the grave. So when mods try to drop their items, there's nothing to drop. Has worked for the most part. Only problem I encountered is that your backpacks will be on drop rule destroy in trinkets when trinket integration is enabled. I've managed to fix it with a mixin however that seems to work without any hassle. Also I don't quite remember but I believe yigd has compat with tb from 1.16.5-1.19.4. I don't maintain 1.16-1.17 anymore (1.18.2-1.19.4 is what I've been working with mostly lately). Also I haven't finished the 1.20 update yet. Part because I decided to rewrite the mod with better code, but also due to a lot of distractions. Compat for 1.20 is planned though

Tiviacz1337 commented 1 year ago

'Only problem I encountered is that your backpacks will be on drop rule destroy in trinkets when trinket integration is enabled.' Yeah that's what I was talking about That I can easily do it from my end. But if everything is working fine, without any bugs etc. we can keep compat on your end, althrough I have no problem to move it to my mod too. But since I support all versions, I'd be glad if you checked if all versions work without problems

B1n-ry commented 6 months ago

Hi again. Sorry for completely ignoring that request for literally half a year. I've checked now and it should be good for all versions from 1.16 up to 1.19. (It works on 1.16.5 - 1.18.1 just because I didn't have the trinket drop rule check implemented yet lol, but 1.18.2 - 1.19.4 are handled with a mixin).

Anyway, I have not yet implemented the trinket integration drop rule compatibility for my 1.20 branch yet. I was checking how it could be fixed, and I think all that is needed is to include YiGD in the isAnyGraveModInstalled() method. Either doing it with a mixin on my end or just straight up on your end would work. The only slight problem would be that if in my mod someone disabled the trinket compatibility or graves as a whole through my configs, the backpack would drop as an item instead of being placed on the ground. I haven't fully elaborated on this idea yet and what could be done (I could probably fix this potential issue on my end), but I wanted to get your thoughts if you had any first. Let me know what you think if you have any ideas or preferations

Tiviacz1337 commented 6 months ago

Sorry, I'm not able to discuss on the issue right now, my schedule is really busy and I'm not able to work on Traveler's Backpack for at least upcoming 2 weeks. But I think I'll just add the deleted method that causes recent crashes and release update with this method. It won't do anything but will be there just for the sake of compatibility and prevent the crash. And if you would decide to fix the issue on your side, it'll work too without any changes necessary on my side.

B1n-ry commented 5 months ago

I've added the trinket integration check myself to the 1.20 version now (just released a new version). I believe that would be the best case as the graves are configurable as to them being enabled or disabled in general. I think that should be compatibility on every version now. Sorry again for the long delay in responses.