Versioned injection - #35
Draft
OrderedSet86 wants to merge 2 commits into
Draft
Conversation
Owner
|
I remember testing it before this changed and it worked, what actual bug are you trying to fix? Do they not change values as expected? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The LLM didn't seem to the existing @versioned much, so it applied these fixes.
You're welcome to override or throw away these changes (I did not test them extensively), but I thought I should post them in case it is useful
@Versionedhas never injected anything. Two independent faults, either alone enough, plus a third that could stop the game loading.Fixed
Class.forName(inner.getSimpleName()), which always throws; the mirror wasskipped before any field was reached, so the existing warning never fired.
@Versioned.Classnow requires the class named in full.static final intwith a literalinitializer is a JLS 4.12.4 constant variable, folded into every use site, so
the write landed where nothing reads. Mirrors are now non-final: reads of
AE2Provider'sTileInscribermirror went from 0getstaticto 4, and itsthree
ConstantValueattributes are gone.injectAllloaded every annotatedprovider regardless, catching only
ReflectiveOperationException- a providernames its mod's types throughout, so the load raises
NoClassDefFoundError.The mod id is now read from the ASM table and checked before anything loads.
setAccessible. AE2'smirrors are public so it never showed; most mods keep constants private.
Unsafeis gone, with its seven[removal]warnings. It was only neededto write a
static final, which mirrors no longer are.Added
@Versioned.Constantis@Repeatable, so one constant can follow a field thedependency renamed: one annotation per name, newest first, first that resolves
wins. Drift is reported against the earliest version any name was expected under.
Tests
VersionedInjectionTest- the mechanism, against a stand-in dependency declaredin the test, so no mod is needed and no assertion drifts into claiming something
about that mod's balance.
VersionedAsmSweepTest- the FML sweep, including that an absent mod's provideris never classloaded.
VersionedMirrorShapeTest- fails if a mirror declares a final field. A sourcecheck, because reflection cannot tell an injected final from a folded one.
Not in scope
Reading a field off every constant of a dependency enum, and reading enum names as
a list. Both are wanted; neither has a caller on
dev.For review
modPresentandinstalledlook like one predicate written twice. They arenot: with no Forge to ask, one must answer "proceed" and the other "do not warn".
@Versioned.Class(clazz = ...)stays a trap for an optional mod, since readinga Class-valued element whose class is absent throws
TypeNotPresentException.The sweep contains that now and the javadoc points at
value(). Say if theelement should go.
TileMolecularAssembler.MAX_PROCESSING_TIMElogs onewarning per launch - AE2 has no such field there. Worth leaving: on the same
class,
ACCELERATION_TAXis absent in 987 and 997 and present in 1017, so amirror missing from the pinned version is the case this annotation exists for.