Skip to content

Config Screen customisation is a bit of a mess #60

@ChiefArug

Description

@ChiefArug

Catnip has a pretty cool config screen system, but it has a few problems wrt to multiple mods trying to customise it.

Namely the shadowState is shared between all mods as it is a static field:

public static BlockState shadowState = Blocks.POTTED_CRIMSON_ROOTS.defaultBlockState();

This means that when Create sets it to a cogwheel that applies to every mod that uses catnip's config screen system. A mod can undo this and set it to their own, but then the same problem happens.
There are also a couple of Maps that mods are expected to register their modid to for customising the screen (both maps in different classes mind you). Neither of these maps is thread safe and honestly a static map is just the wrong way to do it when you have constructor params or inheritance.

My suggestions here would be to rename BaseConfigScreen to ModConfigScreen (just for clarity, as ConfigScreen is actually the base class) and then shift some code around to allow ModConfigScreen to pass a shadow state to ConfigScreen, and have some easily accessible methods available to override things like rendering the background using inheritance.


There is an argument to be made for the static fields as they allow a mod to customise the main config list screen, and potentially some fields could be kept around to set the default one used when a specific mod config is not being viewed, but I do not believe this should be the main way to customise it because static state is pretty messy when you have inheritance and constructor parameters available.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions