MercurialPony / Locks

The source code for Locks, a small, but unique Minecraft mod that introduces flexible and universal locks, a fun new lock picking mechanic and lots of other useful tools.
21 stars 24 forks source link

[1.16.5] Chests spawning with two locks at once #109

Open Darkmega18 opened 3 years ago

Darkmega18 commented 3 years ago

Sometimes in world gen locks on chests are relatively likely to spawn doubled. I'm not sure why. But is there a way you can tick chests to check if they've already got locks on them and prevent a second lock/smash the second lock that somehow magically gets applied to it automatically? it's slightly annoying to see a wood lock, only to then notice theres a shocking diamondone just behind it. :V

I can send a modpack if you want specifics on mods included. But it's for MC 1.16.5 on forge 36.2.4 and using locks 3.0.3.

There are a hefty number of different biome mods, underground generators, cave gen and structure gen mods. Lootr is also in the modpack and converts the chests into special multiplayer loot chests that can be accessed by each person and gives different loot. I have half a guess this might be the reason why it occurs, since all the chests that have accidentally gotten double locks during generation is on a lootr chest. I'm going to report it to them also to see if theres anything they can do incase also.

Zhaltor commented 3 years ago

I don't think it's an issue with Lootr, I get the same problem and I don't use it.

Darkmega18 commented 3 years ago

I don't think it's an issue with Lootr, I get the same problem and I don't use it.

Hmm... do you use other things that change chests in certain ways? like, charm changing the looks of chests etc?

Zhaltor commented 3 years ago

I don't think it's an issue with Lootr, I get the same problem and I don't use it.

Hmm... do you use other things that change chests in certain ways? like, charm changing the looks of chests etc?

I do have Charm. I was not aware it changed anything about chest. Or would adding chest variants be enough to break this mod?

Darkmega18 commented 3 years ago

thats what I'm thinking it could be. Since the chests could get spawned by the game and have their textures/entity replaced by charm or quark or something to change their locks. locks is like: "ok, theres a chest. Locked it. all good!" But then lootr goes "hey, a chest to change into a loot chest so everyone can use it" and changes it to a shiny chest. but then... well, locks is like "oh hey, another chest. locked. yay!" and everyone else is like: -_-'

if it's not lootr, it could just be the case of the chest being replaced by charm and/or quark also. otherwise I got no idea. maybe bursts of lag when generating large structures could do it too. since my pack is a touch on the bigger side when it comes to world gen of structures and loots.

noobanidus commented 3 years ago

But then lootr goes "hey, a chest to change into a loot chest so everyone can use it" and changes it to a shiny chest. but then... well, locks is like "oh hey, another chest. locked. yay!" and everyone else is like: -_-'

I can clarify, that's definitely how Lootr functions in some instances. My apologies, I forgot to subscribe to this issue.

The majority of the functionality is thus:

  1. A TemplateProcessor replaces all eligible chests in structures pre-placement with Lootr equivalents (Villages, Pillager towers, etc)
  2. Specific Igloo processor: manually replaces the chest in the handleDataMarker stage. Most likely causes this issue.
  3. Specific Monster Room processor: hooks into the reorient call and replaces the passed-in default state (usually a chest) with a custom Lootr chest.
  4. Specific Ocean Ruins processor: like the Igloo processor, manually replaces the chest in the handleDataMarker stage. Most likely causes this issue.
  5. Static setLootTable (RandomizableContainerBlockEntity or LockableLootTileEntity depending on version) method: manually replaces the chest that's placed if it's eligible to be replaced. Most likely causes this issue. Additionally, this is generally the most commonly used hook whenever a chest is programmatically placed.
  6. Specific StructurePiece::createChest processor: Like the Monster Room processor, replaces the block state with the Lootr chest block state.

There are three instances where Lootr is definitely removing and then re-placing a tile entity.

If there's a Locks API, I could potentially hook into that and remove a lock whenever I remove a pre-existing check.

Alternately, and depending on how locks are handled (I presume they're on a block-pos/dimension basis, but if they aren't then this point is completely invalid), simply checking to see if a lock has already been associated with that location might be sufficient.

EDIT: Having looked through the source for this mod, it appears to use a chunk capability to store the information, primarily based on bounding boxes. My suggestion here would be to check if the currently-being-placed and potentially-being-locked chest already intersects using LocksUtil::locked or LocksUtil::intersecting.

Additionally, the use of chunk capabilities and location implies the possibility that wrapping a locked chest with a cardboard box or using some other type of tile mover and moving its location or moving it into a different chunk could a) result in errors, or b) result in an "easy" way to unlock chests.

Issues associated with this are why I moved to a UUID-based system instead of using block positions.

Darkmega18 commented 3 years ago

well, I know from testing in my pack I can suck up lootr chests with cyclic, carry on and the bag of yurting. Placing them back down they keep their instanced contents and can still be opened by others.

However if they're initially locked the lock denies any attempts at modification short of very powerful explosives which just obliterate it. Explosive has be to close to a point blank nova cataclysm (projectE) sorta area of explosive. a lot bigger than a typical average grenade or tnt trap that might bust a chest by accident.)

But when trying to suck up chests into a bag of yurting if a lock is properly locked down and actually protecting the chest the surrounding blocks will suck up, but the locked lootr chest won't.

Darkmega18 commented 2 years ago

not until melonslise comes back. :< at best, reduce the chance of locks spawning, so that when lootr or whatever else could be doing this makes them double generate theres less of a chance of both appearing at once. thats what I've done.