Implement machine config simplifier + detect when out of date - #30
Implement machine config simplifier + detect when out of date#30OrderedSet86 wants to merge 21 commits into
Conversation
The machine presets supply these values now.
An absent setting means the machine supplies the value. Applies to +/- buttons
LWJGL 2 (dev) reports 120 per wheel notch. lwjgl3ify (full pack) reports 1. In dev this means the zoom slams between 10% and 500%, which is unusable.
Add (X/N) to slot viewer, fix stale slot name
OrderedSet86
changed the title
OrderedSet86
changed the title
|
First of all, I want to say that I really appreciate the amount of work you are putting in and I know you have not marked it for review, but here are my thoughts about it: Let's start simple, the gui has to be made properly, no clickzones or random stuff, it was originally my mistake to think that we could make a temporary setup and rework it later, I now know it takes more effort to rework it.
"nei_handler" : [
"preset1" : {
"setting1" : 8,
"setting2" : false,
"setting3" : 33%,
},
....
],
"nei_handler2" : [
"preset1" : {
"setting1" : 8,
"setting2" : false,
"setting3" : 33%,
},
....
],
EDIT: After thinking it over I realized that the pack changes rarely, and needing the presents file will probably be overkill, but I would probably still want to have it since a pack may want to define some mixins or something. But this can be done later I would say. One general critique is still valid, don't focus too much on GT, there are also other mods and I would like a system that can be extended easily per mod, also look at the versioned annotation, maybe they could be expanded to query different parameters/values depending on the mod version to keep compatibility with older versions of mods |
| MAX_REGULAR_OC("max_regular_oc", 0, 0, 64, (v, c) -> "Rg" + v), | ||
| MAX_TIER_SKIPS("max_tier_skips", 0, 0, 10, (v, c) -> "Sk" + v), | ||
|
|
||
| FUEL_EFFICIENCY("fuel_efficiency", 100, 1, 1000), |
There was a problem hiding this comment.
Leave all these settings there (ok you can remove FORESTRY_RF_PER_TICK, I thought it was already gone). I want machine presets to change the values of settings/knobs, not replace them
| int maxTierSkips, boolean unlimitedTierSkips, @Nullable RecipeOverride recipeOverride, EnumSet<Knob> knobs) { | ||
|
|
||
| /** Structure inputs a preset reads; the UI shows exactly the rows named here. */ | ||
| public enum Knob { |
There was a problem hiding this comment.
I think this requires me to hardcode values into PlanNH, such as Coils: ["Cupronickel", "Kanthal", "Nichrome", ...]. Is this your preference? I want to avoid doing this for maintenance reasons
There was a problem hiding this comment.
Nevermind, found a way to make it work
sbancuz
left a comment
sbancuz
left a comment
There was a problem hiding this comment.
It seems that GT still has a lot of custom internal structures that could be extracted, and some could be duplicated.
Did not look at all the tests, but I assume most are not needed, only keep the ones that actually test behaviors, not implementation details.
Skim off a bit of comments, I know clankers do love them
| * What a chart persists. Must be stable across game versions and independent of locale, because a | ||
| * chart saved today has to resolve to the same machine on a pack updated tomorrow. | ||
| */ | ||
| @Nonnull |
There was a problem hiding this comment.
These are not needed if you put the package-info everywhere
There was a problem hiding this comment.
If you mean @nonnull, apparently @ParametersAreNonnullByDefault covers parameters only — not return types
|
|
||
| /** The settings this machine reads, which are exactly the rows a node offers for it. */ | ||
| @Nonnull | ||
| Set<Settings> knobs(); |
There was a problem hiding this comment.
While I like the term knobs, it may a bit confusing since they are just settings. Don't like having 2 names for the same exact thing
There was a problem hiding this comment.
replaced by this rule:
| * recipe list's tab title. | ||
| */ | ||
| @Nonnull | ||
| default String label() { |
There was a problem hiding this comment.
Why not just use the display name directly?
There was a problem hiding this comment.
This is because singleblocks are constructed as "[display name] (single)"
| * cached instance until its answer actually changes. | ||
| */ | ||
| @Nullable | ||
| private static List<? extends MachineVariant> lastCandidates; |
| return TIERS.size(); | ||
| } | ||
|
|
||
| public static int highestTier() { |
There was a problem hiding this comment.
Remove this, rename count to numTiers or something
| * rename fails the build rather than a player's chart. It counts these members too, so a field added | ||
| * here without a row there fails as well. | ||
| */ | ||
| final class ProbeFields { |
There was a problem hiding this comment.
There already is FieldInjector, why another class that uses reflection to get some values?
| * Two readings are compared whole, because {@link ProbeReading} is a record. Any difference in | ||
| * parallel count, speed, power, overclock behaviour or heat counts as the knob mattering. | ||
| */ | ||
| public final class SensitivityScan { |
There was a problem hiding this comment.
This class seems useless, it's only used in the SensitivityScanTest (I think)
| private static final String DEPRECATED_LINE = "GT5U.MBTT.Deprecated.NEI"; | ||
|
|
||
| /** How a machine is built, which is what decides whether voltage and parallels are editable. */ | ||
| public enum Kind { |
There was a problem hiding this comment.
Can't you use the already computed setting for this? Or is that calculated from here?
| * node then selected out of this list is memoized once, in MachineVariants. | ||
| */ | ||
| @Nullable | ||
| private static RecipeMap<?> lastMap; |
| if (entry.preset() == null) return 0; | ||
| return entry.preset() | ||
| .maxParallel() | ||
| .applyAsInt(new StructureState(5, 5, 4, 4, 2, 0, 0, 1, 0, 0)); |
There was a problem hiding this comment.
What? Where do these numbers come from?
There was a problem hiding this comment.
This functionality is poorly named/documented, this is part of machinery to probe the parallel count of the machines so it knows how to order the variant dropdown by "complexity." I will make the function clearer
|
To minimize the scope of this PR, I am going to avoid adding new machines until later PRs. Most of the common ones are covered by the current inclusion set. The infrastructure/framework will still be generalized, but this avoids having to hand check 138 more GT machines before merge |
Do you plan to reopen multiple new stacked prs or do you have another idea? |
2 participants
Machine alternative selection dropdown
Example trimmed machine config
Per-chart minimums
Config knobs tuned per machine by code inspection. Lookup table generated with
/plannh_machinesin game on./gradlew runClient -PgtnhRecipes:PlanNH: what it knows about each GregTech multiblock
Columns
(Tier, Mach, Amp, Advanced). Blank means it asks nothing else, which is the goal.
Blank means none do, so the node offers no structure rows at all.
a -> bmeans thehand-written row claims
aand the machine itself reportsb, which is a row to fix.{map=n}says whichmode each recipemap selects, so the node derives it and asks nothing;
asksmeans therecipe cannot say which mode is meant and the node still offers the row.
both (32)
Both a hand-written row and a probe reading. The row is what a chart uses; the probe is the check on it. Anything in the knobs column with an arrow, or any machine listed under a differing headline in the log, is a row that no longer matches GregTech.
probe (99)
Read from the installed GregTech at runtime, with no hand-written row behind it. These are machines the table never covered, and they follow whatever GregTech the pack ships without anybody maintaining them.
describer (24)
GregTech supplies its own overclock behaviour for these, and it outranks every preset - GTPresetApplier uses the describer in preference to any row or probe reading. So a row here would change nothing.
override (3)
A hand-written row kept deliberately, because the probe cannot answer or answers wrongly. The reason is on each line and in GTMachineOverrides. These are the only machines where PlanNH still asserts a number against the machine that owns it.
unmodelled (138)
Nothing models these. They plan as a plain single-speed machine with no parallels and no overclock behaviour, which for a generator or a bespoke endgame multiblock may be the honest answer, and for the rest is the work queue.