Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added configuration screen #1199

Merged
merged 45 commits into from
Jul 19, 2024
Merged

Added configuration screen #1199

merged 45 commits into from
Jul 19, 2024

Conversation

HenryLoenwind
Copy link
Contributor

@HenryLoenwind HenryLoenwind commented Jun 29, 2024

It's not completely finished, but very close to a state I'd stop with.

This re-adds a generic configuration UI that can show and edit all the values the configuration system can handle. It is opt-in, and mods can use it as a base class to do their own thing. It mostly uses vanilla's configuration system with a couple of tweaks on top.

There are no breaking changes. It is mostly self-contained, with the exception of an addition to how lists are configured. For one, it allows mods to restrict the lengths of lists (in both directions), but more importantly, it gives the UI a way of creating new elements to add to lists.

I also fixed 2 configuration values that still had "forge" in the code but "neoforge" in the language file.

Open TODOs:

  • Warning screen after changing values that require a world/client restart (important)
  • "Reset to default" button
  • "Undo" button
  • More javadoc
  • PR to documentation repo
  • Narrator for custom widget (help wanted as tagged in the source)
  • Edit dedicated server config (out of scope)
  • Discarded: Visually mark changed values (vs default/vs original value)

To use this in a mod, add container.registerExtensionPoint(IConfigScreenFactory.class, ConfigurationScreen::new); to your client-mod class constructor. (Although this probably works fine in the normal mod class, but the advanced constructor wouldn't.)

image
image
image
image
image
image

@neoforged-pr-publishing
Copy link

neoforged-pr-publishing bot commented Jun 29, 2024

  • Publish PR to GitHub Packages

Last commit published: d65fea96c1d5cb88fba78811942bce4f308fde97.

PR Publishing

The artifacts published by this PR:

Repository Declaration

In order to use the artifacts published by the PR, add the following repository to your buildscript:

repositories {
    maven {
        name 'Maven for PR #1199' // https://github.com/neoforged/NeoForge/pull/1199
        url 'https://prmaven.neoforged.net/NeoForge/pr1199'
        content {
            includeModule('net.neoforged', 'testframework')
            includeModule('net.neoforged', 'neoforge')
        }
    }
}

MDK installation

In order to setup a MDK using the latest PR version, run the following commands in a terminal.
The script works on both *nix and Windows as long as you have the JDK bin folder on the path.
The script will clone the MDK in a folder named NeoForge-pr1199.
On Powershell you will need to remove the -L flag from the curl invocation.

mkdir NeoForge-pr1199
cd NeoForge-pr1199
curl -L https://prmaven.neoforged.net/NeoForge/pr1199/net/neoforged/neoforge/21.0.155-beta-pr-1199-ui/mdk-pr1199.zip -o mdk.zip
jar xf mdk.zip
rm mdk.zip || del mdk.zip

To test a production environment, you can download the installer from here.

@CLAassistant
Copy link

CLAassistant commented Jun 29, 2024

CLA assistant check
All committers have signed the CLA.

@neoforged-compatibility-checks
Copy link

neoforged-compatibility-checks bot commented Jun 29, 2024

@HenryLoenwind, this PR introduces breaking changes.
Fortunately, this project is currently accepting breaking changes, but if they are not intentional, please revert them.
Last checked commit: d06e50c7628ab7868f996e4dc59ae7b8bbfa4a62.

neoforge (:neoforge)

  • net/neoforged/neoforge/client/gui/IConfigScreenFactory
    • createScreen(Lnet/minecraft/client/Minecraft;Lnet/minecraft/client/gui/screens/Screen;)Lnet/minecraft/client/gui/screens/Screen;: ❗ API method was removed
    • createScreen(Lnet/neoforged/fml/ModContainer;Lnet/minecraft/client/gui/screens/Screen;)Lnet/minecraft/client/gui/screens/Screen;: ❗ Method was made abstract
  • net/neoforged/neoforge/client/gui/ModListScreen$SortType
    • compare(Lnet/neoforged/neoforgespi/language/IModInfo;Lnet/neoforged/neoforgespi/language/IModInfo;)I: ❗ API method was removed
  • net/neoforged/neoforge/client/ClientNeoForgeMod
    • <init>(Lnet/neoforged/bus/api/IEventBus;)V: ❗ API method was removed

@Matyrobbrt Matyrobbrt added enhancement New (or improvement to existing) feature or request help wanted Extra attention is needed 1.21 Targeted at Minecraft 1.21 labels Jun 29, 2024
@HenryLoenwind
Copy link
Contributor Author

Also, if someone would clean up neoforge's own configuration in regards to which ones are missing entries in the language file and which entries in the language file are zombies from old versions, that'd be great.

For this, note that the UI tries to get the tooltip from the language file and only uses the comment as fallback. While this doubles the text into two places, it also allows the tooltip to be translated and the comment to be present on servers.

@IchHabeHunger54
Copy link
Member

Hello from the docs team!

The entire config article still needs a rework, so documentation for this system should best be added alongside that rework as a whole. In other words, don't let a missing docs PR stop you from merging this.

People put all kinds of stuff into the system, like List<List<Object>> = { {String, Number}, ... }. Better at least show the user that there is something or they might think we're broken.
Can't edit values that have been synced to others or could be synced in the middle of editing.
@HenryLoenwind HenryLoenwind marked this pull request as ready for review July 1, 2024 21:01
@XFactHD XFactHD linked an issue Jul 13, 2024 that may be closed by this pull request
@TelepathicGrunt
Copy link
Contributor

I am approving this PR based off of end modder behavior and not the code portion. Just be sure to add to PR description that modders gets a console output of untranslated keys only if they visit the screens with the untranslated stuff and then exit the config gui. This should be stated so modders know this behavior exists.

Narrator support could be a future PR

@TelepathicGrunt
Copy link
Contributor

@HenryLoenwind Looks like there's a conflicting file rn

@TelepathicGrunt TelepathicGrunt merged commit 7d465ab into neoforged:1.21.x Jul 19, 2024
5 checks passed
Comment on lines -27 to +26
Screen createScreen(Minecraft minecraft, Screen modListScreen);
Screen createScreen(ModContainer container, Screen modListScreen);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HenryLoenwind This is a (small) breaking change, please include the breaking change tag on this commit for visibility or add a backward compatible method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Matyrobbrt Your change ^

@TelepathicGrunt TelepathicGrunt added the breaking change Breaks binary compatibility label Jul 22, 2024
mezz added a commit to mezz/JustEnoughItems that referenced this pull request Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.21 Targeted at Minecraft 1.21 breaking change Breaks binary compatibility enhancement New (or improvement to existing) feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config GUI
7 participants