Skip to content

Clear block state map before recreating it during bake.#15

Merged
BluSpring merged 2 commits into
KiltMC:kilt/1.21.1from
Acuadragon100:kilt/1.21.1-block-map-duplicates
Jun 7, 2026
Merged

Clear block state map before recreating it during bake.#15
BluSpring merged 2 commits into
KiltMC:kilt/1.21.1from
Acuadragon100:kilt/1.21.1-block-map-duplicates

Conversation

@Acuadragon100

Copy link
Copy Markdown

Fix for players being unable to join servers.

@BluSpring

Copy link
Copy Markdown
Member

I don't think this patch is valid, the actual cause of the problem is more likely due to onClear never getting called, which would be doing pretty much what you are doing except differently. I'm pretty sure it's never called since Kilt doesn't really utilize Neo's configuration tasks (including registry resync), which is intentional. You should look into getting onClear / Registry#clear getting called but using Fabric's registry sync code instead.

@Acuadragon100

Copy link
Copy Markdown
Author

On second thought, aren't we doing some things twice at the moment?
I mean, NeoForgeRegistryCallbacks.BlockCallbacks does a bunch of things which Fabric API already takes care of. The main thing it does which Fabric API does not is handle the clear function for the registry (although BlockCallbacks also runs BlockStateBase::initCache for all block states, but this happens during world load via Blocks::rebuildCache anyway).
Maybe we should just disable BlockCallbacks::onBake completely?

@BluSpring

Copy link
Copy Markdown
Member

I have seen some weird shit happen from disabling stuff, and since things seem to work for the most part at the moment it should be fine to keep around.
Also, unfortunately, some mods (Architectury API-based mods) do seem to actually utilize the behaviour that BlockCallbacks::onBake is intended to deal with.

@Acuadragon100

Acuadragon100 commented Jun 6, 2026

Copy link
Copy Markdown
Author

Fair enough, but Block::BLOCK_STATE_REGISTRY is filled twice on the server side as well. I don't think a registry sync will deal with that, so we probably still need to clear Block::BLOCK_STATE_REGISTRY manually before BakeCallback::onBake runs. I don't think we want to do that via ClearCallback::onClear because other mods might not expect that. The best place I can think of doing it would be either in MappedRegistry::freeze, or directly in BlockCallbacks::onBake (Fabric API fills Block::BLOCK_STATE_REGISTRY via RegistryEntryAddedCallback).

Honestly, the more I think about it, the more this patch feels like the best solution.

@BluSpring

Copy link
Copy Markdown
Member

I feel like either way onClear still needs to be called somewhere, from what I can gather it's not getting called at the moment.

@BluSpring

Copy link
Copy Markdown
Member

If anything, we can honestly limit the clearing in the MappedRegistryInject so we don't cause problems to other mods.

@Acuadragon100

Copy link
Copy Markdown
Author

I feel like either way onClear still needs to be called somewhere, from what I can gather it's not getting called at the moment.

As far as I can see, clear is only used from RegistryManager::applySnapshot, which is only really used by the NeoForge registry sync or when errors occur.

@BluSpring

Copy link
Copy Markdown
Member

Yeah, we really need to call it, because some Neo mods hook into the onClear event that is dispatched by these registries.

@Acuadragon100

Acuadragon100 commented Jun 6, 2026

Copy link
Copy Markdown
Author

Ah, I see.
Do you think we should run RegistryManager::applySnapshot from a RegistryIdRemapCallback? We could limit it to only vanilla registries and registries with a NeoForge mod id. I was thinking we could add phase ordering so it runs after most events.

For reference, it might look something like this:
https://github.com/KiltMC/Kilt/compare/version/1.21.1...Acuadragon100:version/1.21.1-clear-on-registry-sync?expand=1

@BluSpring

Copy link
Copy Markdown
Member

Personally not a big fan of applySnapshot and I'm not sure if it'll find any use.

@Acuadragon100

Copy link
Copy Markdown
Author

Hmm...
In that case I'm not really sure about the best way to do this. Would you prefer just running onClear and onBake directly in the RegistryIdRemapCallback?

@BluSpring

Copy link
Copy Markdown
Member

Might be our best bet here, to be honest.

@Acuadragon100

Copy link
Copy Markdown
Author

Might be our best bet here, to be honest.

Do we want to do that for Fabric-specific registries as well, or only vanilla/NeoForge ones?

@BluSpring

Copy link
Copy Markdown
Member

Best to track for all, since we're still bridging it.

@BluSpring BluSpring merged commit 55f08a1 into KiltMC:kilt/1.21.1 Jun 7, 2026
1 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants