Lost Trinkets

Lost Trinkets

8M Downloads

Blacklisted trinkets still unlocked and overwrote two other trinkets when disabled

AzureMonument opened this issue ยท 7 comments

commented

So I've been playing a survival with this mod, and got two trinkets I feel are way too overpowered- ash gloves and magical feathers. So I found the config and blacklisted them, as well as prevented from randomly unlocking.

Ssatisfied, I went back into my world, but the trinkets were still there! Confused I tried restarting the game entirely, and that didn't to do anything. I saw lolipop updated, so I grabbed that (version 3.2.7 now). So I log in, and at first the trinkets are gone. Satisifed I selected golden tooth, and tha spider. But suddenly they turned into ash hand and magical feathers!

image
image

As you can see here, golden tooth and tha spider are gone, while ash hands and magical feathers are now equipped (magic feathers is in the second image as I had to remove it to get to the selection screen.)

My config file is as follows:
#List of banned trinkets eg: ["losttrinkets:piggy", "losttrinkets:magical_feathers"]
#The trinkets listed in here will also be removed from players that already unlocked them.
blackList = ["losttrinkets:magical_feathers", "losttrinkets:ash_gloves"]
#List of trinkets that can't be unlocked randomly eg: ["losttrinkets:piggy", "losttrinkets:magical_feathers"]
#The trinkets listed in here will not be removed from players that already unlocked them.
nonRandom = ["losttrinkets:magical_feathers", "losttrinkets:ash_gloves"]

I'm currently in 1.16.5, with lost trinkets version 0.1.22 & lolipop version 3.2.7

commented

Is this singleplayer or multiplayer?
Maybe you can you try with the trinkets just in blackList but not in nonRandom? You only need them in one of the lists.
Also, yeah, you have to restart the game completely to apply config changes for Lost Trinkets at the moment, you can't just edit the config then reload the world unfortunately. :(
Banned trinkets get removed when you log into a world I think, which is like you observed.

As for the trinkets transforming to another trinket, that shouldn't be possible afaict, which leads me to think it might be a desync of some kind or maybe if it's a server then the config doesn't match up with the server if it's multiplayer or something?
I couldn't reproduce this happening...
It would be good if you could come up with exact steps with a fresh world and a config that causes it to happen.
Like what trinkets to spawn in, how to change the config, restarting the whole game, then what to do after starting up the world again.

Steps I tried:

  • With no changes to configs, create a new super flat world in creative
  • Spawn in Ash Gloves, Magical Feathers, Golden Tooth, Tha Spider
  • Switch to survival, and use each of them to unlock them
  • Give experience with /experience add <name> 1000 levels
  • Unlock one slot (total of 2 slots)
  • Equip Ash Gloves and Magical Feathers
  • Close the game completely
  • Edit config to have blackList = ["losttrinkets:magical_feathers", "losttrinkets:ash_gloves"]
  • Open the game again and log into the world
  • Open trinkets screen: both slots empty
  • Click on empty slot: see Golden Tooth and Tha Spider
  • Choose Golden Tooth: see Golden Tooth equipped
  • Click on empty slot: see Tha Spider only
  • Choose Tha Spider: see Golden Tooth and Tha Spider equipped
  • Exit world and re-open world
  • Open trinkets screen: see Golden Tooth and Tha Spider equipped

So yeah, can't reproduce what happened to you. The trinkets didn't transform to the wrong ones.
I also repeated the above with having blackList and nonRandom both having the same values and the same thing happened.

commented

This is singleplayer, it seems to work fine with only blacklist enabled. I have no clue how to replicate it if you haven't found it with those steps. I tried again and managed to delete magneto and lunch bag.

EDIT: I've been mining trees and just re-unlocked both magneto and luck coin, just as proof that I'm not making this up.
image
EDIT 2: I am player with other mods, so it may just be a compat or an issue with saving.

EDIT 3: Wait, I just missed them, the magical feathers and ash glove are still here!
image

commented

It's not that I don't believe it happening to you. It's that I need to be able to reproduce it on my end to be able to look into the bug.
Can you try follow my steps to see if it happens for you, using just LostTrinkets and Lollipop?
Also please provide your steps to make it happen on fresh client with just Lollipop and Lost Trinkets and no other mods except maybe JEI.
Also please provide debug logs when it happens again. You can find it in ./logs/debug.log.

Edit: I don't think it's a mod conflict because afaict Lost Trinkets handles it seperate from other inventory systems and such with a capability that I don't think other mods are going to touch it.
But yeah, we need more information to be able to fix this.

commented

I'm having a similar issue - running just trinkets, I tried blacklisting the below trinkets, then using nonRandom, and then both. The issue is the same: if you, for example, pull them out of creative/JEI and unlock them, you can still select and equip them whether you're in survival or creative. They don't go away until the player relogs. The NonRandom list does appear to prevent the items in that list from showing up, and I killed a looooot of zombies to test it.

I think the solution to the blacklist is to prevent the items being registered in the first place, which should remove them from the player's NBT data. That might need some testing however, could cause issues if you don't do a sanity pass on wherever you're storing your trinkets in player data. That, or run a check of the equipped items when you open/close the menu or attempt to equip something. That's probably the easiest way.

I don't have a debug.log (likely because I'm not in a dev environment) but I do have a couple latest.log with the blacklisted items I set:
https://controlc.com/a01bb0a8
https://controlc.com/7c5738a4

commented

pull them out of creative/JEI and unlock them, you can still select and equip them whether you're in survival or creative. They don't go away until the player relogs.

Yes. This is how it works currently. I suppose we could change it to only working in creative though.

The NonRandom list does appear to prevent the items in that list from showing up, and I killed a looooot of zombies to test it.

nonRandom list does prevent the trinkets from being randomly given for me. Your logs don't show the trinkets being nonRandom. If it is in the nonRandom list but not in the blackList then it should print something like:

Not Unlockable Trinket: <trinket resource location>

I did not see lines like that in both of logs given (only blacklisted ones). Are you sure one of two tests (assuming two because two log files given) was done of them with only nonRandom and not blackList set?

I think the solution to the blacklist is to prevent the items being registered in the first place, which should remove them from the player's NBT data.

We don't prevent the actual items themselves from being registered, because of the way current Forge works, the registry events are run before configs are loaded and it's recommended to always register everything and instead disable functionality, which we have done by disabling the unlocking of the trinkets.

  • Trinkets in the blacklist can't be randomly obtained.
  • The trinkets are removed when the player logs in.
  • The config change requires the game to be restarted (so players will have to log back in to play).

Given all of this, how can they get the trinkets without spawning them in?

The only way I can think of is if a pack maker added a recipe for them but forgot to remove it, or added it as loot but removed it in a later update.
I still think it's fine cause then players would need to have a trinket that they want to use from before the update and stock pile it, then they'd consume one each time they log in. But we can prevent black listed trinkets from being used like mentioned above. At which point they're going against the pack maker's vision anyway so are ruining it for themselves.

I'm having a similar issue

Well yeah, your post doesn't give any extra information for how to solve this issue and really should be a seperate one.
Please don't do this in the future.

Anyways, I'll do a PR to prevent trinkets in the blackList from having their item form be usable to unlock outside of creative mode at some point.

commented

Sorry for the late reply. I should have provided a disclaimer, I'm not a mod dev and if I misused a phrase like register vs disabling functionality, apologies for the confusion. The main point I was trying to get across is I had an assumption that the blacklist would prevent them from being accessible period, not just for survival. That's just a disconnect between my assumption and your intent, which you've addressed.

Now that I understand your design intent better, my concern is less valid as only mods like Lucky Blocks or other chance mods or Quest mods can provide these items as rewards. To the former, absolutely an edge case of one infinitesimally small way a player could obtain these despite the blacklist, which would be only be available long-term if they never log off. For things like quest, that would just be an issue with the curator and not your concern, acknowledged.

For the logs, my first test was blacklist only, the second, longer test was using a combined blacklist and nonrandom - again, another apology for being unclear. I am absolutely certain nonRandom was filled - my config, below. I spent roughly 40 minutes farming zombies with killing = 2 and coldown = 0. After 15~ minutes of not receiving any further unlocks, and verifying that I had all the unlocks except the ones I had in blacklist/nonRandom, as previously mentioned I did conclude the nonRandom config entry was working as advertised using only Lost Trinkets and its library mod.

blackList = ["losttrinkets:magical_feathers", "losttrinkets:magneto", "losttrinkets:rock_candy", "losttrinkets:blaze_heart", "losttrinkets:fire_mind", "losttrinkets:ember", "losttrinkets:mossy_ring", "losttrinkets:mossy_belt", "losttrinkets:ash_gloves"]

nonRandom = ["losttrinkets:magical_feathers", "losttrinkets:magneto", "losttrinkets:rock_candy", "losttrinkets:blaze_heart", "losttrinkets:fire_mind", "losttrinkets:ember", "losttrinkets:mossy_ring", "losttrinkets:mossy_belt", "losttrinkets:ash_gloves"]

To the similarity issue, as mentioned before this was a disconnect between assumption and expectation which appeared related. Because of that disconnect, the solutions/recommendations I did offer were not fully relevant, though I do believe based on this conversation the solutions we've discussed may indirectly correct some of the OP's problems with the blacklist.

Thank you for your replies and added information and fix consideration; this has helped me better reframe my understanding of your mod and hopefully this discussion has added value to you and your mod.

EDIT I forgot to mention which versions I used for my testing:
MC: 1.16.5
Forge: 36.0.46
Lost Trinkets: 1.16.4-0.1.23
Lollipop: 1.16.4-3.2.8

commented

For the logs, my first test was blacklist only, the second, longer test was using a combined blacklist and nonrandom

Yes, if you have something in both the blackList and nonRandom it's redundant because the blackList applies first, filtering out the trinkets to a set of enabled trinkets, then the nonRandom filters out the set of enabled trinkets into a set of randomly unlockable trinkets. So testing with blackList only and both (assuming the same contents) should theoretically perform exactly the same. You'll want to test with the blackList empty and then have it in nonRandom only.

The main point I was trying to get across is I had an assumption that the blacklist would prevent them from being accessible period, not just for survival.

This was a suggestion for a potential change not something that's confirmed to be intended. That may very well be what we end up doing.
I'm looking the implementation and we don't tick trinkets or call targeting events for trinkets that aren't enabled so certain trinkets wouldn't work properly if unlocked by spawning them in when they are disabled so because of this confusion I think we should just prevent the disabled trinkets from being used in the first place. Perhaps also looking into a way to allow reloading of the configs instead of having to restart the game. Either that or we do tick trinkets and raise events regardless of if they are enabled or not. Anyways this is currently speculation of implementation details.

To the similarity issue, as mentioned before this was a disconnect between assumption and expectation which appeared related.

Your issue, as far as I can tell, is that trinkets can be obtained even though they are in the blackList by spawning them in and using them.
This issue is meant to be for trinkets being overwritten or randomly unlockable while being in the blackList.
IMO that's unrelated to this issue regardless of what the intent is?

TBH though, I think your testing acts as an example where this issue isn't reproducable though.
So that part makes it relevant to some extent.
Because I cannot reproduce this issue happening, and if you can't either then it's on the OP or someone else to to come up with an example of how to get it to happen.

Your issue should definitely be a seperate one though (and then you could post on this issue saying that it's not reproduceable seperately potentially with a link to that seperate issue) because even if/when we make the change to prevent trinkets being useable to unlock them when they are in the blackList, we still couldn't technically close this issue because it's not resolved.

Do note your posts seem to consist of two parts. The issue of being able to right click spawned in trinkets to unlock them and the testing. The former is unrelated, the latter is relevant and useful. Thank you for that testing.

Honestly if it was up to me I'd just label this issue as requires more information then just leave it and close it if there's no response.

I'm thinking, probably just going to implement preventing disabled trinkets from being usable, and then closing this issue after that since enough things would probably change that it might fix this issue accidentally or even if it doesn't the OP hasn't responded with a reproducable example for two weeks, and they can also reopen or someone else can make a new issue if more information comes to light.