Conversation
fix: pattern collisions
fix: array return and type safety on field exprs
add: CondPacketTypeIsBound
add: ExprFields, setter safety
add: getPacket method without side
fix: toString for types
remove: ExprFakeEntityEventValue.java (no longer used)
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR reorganizes entity/meta code into entity-specific packages, replaces global validators with per-schema validators, refactors property expressions to resolve/cached FieldAccessors, updates packet/type registries and serializers, revises many examples, and bumps the project to 1.1.3. ChangesEntity Meta Architecture & Type System Reorganization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Qodana for JVM8 new problems were found
View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2025.3.2
with:
upload-result: trueContact Qodana teamContact us at qodana-support@jetbrains.com
|
There was a problem hiding this comment.
Pull request overview
This PR appears to be the v1.1.3 release bump for PacketEventsSK, combining version updates with a refactor of field/meta registries, new/updated Skript elements, and improved serialization/documentation/examples for several types.
Changes:
- Bump project version to 1.1.3 and update SkriptRegistration dependency.
- Refactor/move packet field registries and entity meta/fake-entity field systems into clearer packages + update registrations accordingly.
- Add new Skript elements and behavior improvements (e.g., packet bound condition, new packet fields, serializers, expanded examples).
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/config.yml | Updates displayed config header version to v1.1.3. |
| build.gradle.kts | Bumps project version and SkriptRegistration dependency. |
| src/main/java/dev/threeadd/packeteventssk/api/general/packet/PacketTypeRegistry.java | Renames internal maps and adds a direction-agnostic getPacket(String) lookup. |
| src/main/java/dev/threeadd/packeteventssk/api/simple/PlayerSkinManager.java | Removes debug logger usage. |
| src/main/java/dev/threeadd/packeteventssk/element/general/GeneralElementRegistration.java | Adjusts registrations for moved/removed meta-related elements; adds packet bound condition registration. |
| src/main/java/dev/threeadd/packeteventssk/element/general/Types.java | Adds converters, serializers (Skin/BlockEntityType), and improves examples + string representations. |
| src/main/java/dev/threeadd/packeteventssk/element/general/condition/CondPacketTypeIsBound.java | Adds condition to check whether a packet type is clientbound/serverbound. |
| src/main/java/dev/threeadd/packeteventssk/element/general/expression/prop/ExprPacketField.java | Refactors packet field access logic and return typing (but introduces correctness issues). |
| src/main/java/dev/threeadd/packeteventssk/element/general/section/SecExprNewPacket.java | Switches to per-schema validators and improves docs/examples. |
| src/main/java/dev/threeadd/packeteventssk/element/general/field/PacketFieldRegistry.java | Moves registry package and maintains registrar wiring. |
| src/main/java/dev/threeadd/packeteventssk/element/general/field/ClientBoundPacketFieldRegistrar.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/general/field/ServerBoundPacketFieldRegistrar.java | Package move + adds UPDATE_SIGN packet fields (but currently has a construction key bug). |
| src/main/java/dev/threeadd/packeteventssk/element/general/field/SkBeePacketFieldRegistrar.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/EntityElementRegistration.java | Registers meta registry/section/expression under entity module; removes fake-entity event value expr registration. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/Types.java | Updates examples; reintroduces/registers EntityMeta type and converter. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/section/SecExprNewFakeEntity.java | Per-schema validators + improved docs/examples and field inheritance formatting. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/section/SecExprNewMeta.java | Moved package + per-schema validators + improved docs/examples and inheritance formatting. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntitiesAll.java | Updates examples to new fake entity section syntax. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityFromId.java | Updates examples to new fake entity section syntax. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityFromUuid.java | Updates examples to new fake entity section syntax. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprFakeEntityField.java | Refactors fake entity field access and docs (but introduces correctness issues). |
| src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprMetaField.java | Reintroduces meta field expression under entity module (but introduces correctness issues). |
| src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprVisibleFakeEntities.java | Updates examples to new fake entity section syntax. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/living/ExprFakeLivingEntityAttribute.java | Updates pattern/examples; changes attribute defaulting behavior. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/field/entity/FakeBaseEntityFieldRegistrar.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/field/entity/FakeEntityFieldRegistry.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/field/entity/FakePlayerEntityFieldRegistrar.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/BaseMetaFieldRegistrar.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/DisplayMetaFieldRegistrar.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/InteractionMetaFieldRegistrar.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/ItemMetaFieldRegistrar.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/LivingMetaFieldRegistrar.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/MetaFieldRegistry.java | Package move only. |
| src/main/java/dev/threeadd/packeteventssk/element/general/expression/prop/ExprMetaField.java | Removes the old meta field expression from the general module. |
| src/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityEventValue.java | Removes the fake-entity event value expression. |
Comments suppressed due to low confidence (2)
src/main/java/dev/threeadd/packeteventssk/element/general/field/ServerBoundPacketFieldRegistrar.java:66
- In the UPDATE_SIGN packet schema, the Vector field is registered as "location vector" (aliases: "location"), but the constructor looks up a required field named "block position". Since ConstructionContext asserts the accessor exists, this will throw at runtime when constructing the packet. Use the registered field name (or one of its aliases) consistently in the constructor lookup.
src/main/java/dev/threeadd/packeteventssk/element/general/field/ServerBoundPacketFieldRegistrar.java:10 WrapperPlayServerOpenSignEditoris imported but not used in this class. Remove the unused import to avoid warnings and keep the file tidy.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
src/main/java/dev/threeadd/packeteventssk/element/entity/section/SecExprNewFakeEntity.java (1)
33-47: ⚡ Quick winClear validator cache before rebuilding.
VALIDATORSis static and repopulated on eachregister()call. Clearing it first avoids stale schema-key entries on repeated registration paths.Suggested change
public static void register(Registration reg) { + VALIDATORS.clear(); for (FieldSchema<EntityType, WrapperEntity> def : FakeEntityFieldRegistry.INSTANCE.getAllSchemas()) { SimpleEntryValidator builder = SimpleEntryValidator.builder();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/threeadd/packeteventssk/element/entity/section/SecExprNewFakeEntity.java` around lines 33 - 47, The static VALIDATORS map is repopulated in register() without clearing old entries, which can leave stale keys; modify the register() method to clear VALIDATORS at the start (e.g., call VALIDATORS.clear()) before iterating FakeEntityFieldRegistry.INSTANCE.getAllSchemas() and rebuilding validators with SimpleEntryValidator.builder(), so the map only contains current FieldSchema keys and their corresponding EntryValidator instances.src/main/java/dev/threeadd/packeteventssk/element/general/section/SecExprNewPacket.java (1)
29-44: ⚡ Quick winReset static validators before population.
Because
VALIDATORSis static, clearing it at the start ofregister()prevents stale schema-validator entries across repeated registration calls.Suggested change
public static void register(Registration reg) { + VALIDATORS.clear(); for (FieldSchema<PacketTypeCommon, PacketWrapper<?>> def : PacketFieldRegistry.INSTANCE.getAllSchemas()) { SimpleEntryValidator builder = SimpleEntryValidator.builder();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/threeadd/packeteventssk/element/general/section/SecExprNewPacket.java` around lines 29 - 44, The static VALIDATORS map is never cleared so repeated calls to register() can leave stale entries; at the start of the register(Registration reg) method clear VALIDATORS (e.g., VALIDATORS.clear()) before iterating PacketFieldRegistry.INSTANCE.getAllSchemas() so you rebuild validators from scratch for each registration; update the register method that references VALIDATORS, SimpleEntryValidator.builder(), and PacketFieldRegistry to perform the clear first.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityFromUuid.java`:
- Around line 31-37: Update the example to use a UUID-specific list variable
name: replace all occurrences of the list variable {-id::*} with {-uuid::*} in
the example snippet inside ExprFakeEntityFromUuid (so the store line "add the
fake entity uuid of {_player} to {-id::*}" and the command lookup loop/contains
lines referencing {-id::*} all use {-uuid::*} instead) to make the intent clear
and avoid confusion.
In
`@src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprFakeEntityField.java`:
- Around line 172-179: The acceptChange() logic in ExprFakeEntityField
mistakenly returns null for array-backed fields because it compares expected
(fieldAccessor.expectedType()) to the component type and only returns when
equal; instead, modify acceptChange() in class ExprFakeEntityField to
unconditionally return an array containing typeToAccept (the component type for
arrays or the expected type for non-arrays) so SET operations are allowed — this
aligns acceptChange() with the existing change() behavior that constructs the
target array.
In
`@src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprMetaField.java`:
- Around line 166-173: The acceptChange logic in ExprMetaField currently returns
null for array-typed fields by comparing expected vs component type, which
blocks SET for arrays; update acceptChange to return the field's expected type
(the array class when expected.isArray()) instead of null so array assignments
are accepted (use fieldAccessor.expectedType() as the returned Class<?> value);
ensure this change aligns with the existing change() handling for array
assignment.
In
`@src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/living/ExprFakeLivingEntityAttribute.java`:
- Around line 66-77: getAttributeValue currently can return null for
non-WrapperLivingEntity, violating the non-null Number contract; change it so it
always returns a numeric default: lookup the PacketEvents attribute
(Attributes.getByName(bukkitAttr.getKey().asString())) outside or before the
WrapperLivingEntity check (or duplicate the lookup in the else branch) and
return its getDefaultValue() when the entity is not a WrapperLivingEntity or
when no Property is found, ensuring getAttributeValue never returns null;
reference getAttributeValue, WrapperEntity, WrapperLivingEntity,
Attributes.getByName, Property.getValue and peAttr.getDefaultValue() when making
the change.
In
`@src/main/java/dev/threeadd/packeteventssk/element/general/expression/prop/ExprPacketField.java`:
- Around line 90-104: The current global-path logic in ExprPacketField binds
this.fieldAccessor to the first matching schema (via
PacketFieldRegistry.INSTANCE.getAllSchemas()), which is unsafe because field
names are reused across schemas; instead, only validate that a schema contains
the field name but do NOT cache an accessor globally. Leave the parsed fieldName
as-is, remove assigning this.fieldAccessor in the global branch, and update any
runtime lookup code that uses fieldAccessor (methods in ExprPacketField that
access packet values from a PacketWrapper) to resolve the accessor per-wrapper
by fetching the wrapper's actual schema (FieldSchema for the wrapper's packet
type) and calling schema.getAccessor(fieldName) for that wrapper each time.
Ensure you reference PacketFieldRegistry, FieldSchema, PacketWrapper, fieldName
and fieldAccessor when making these changes.
- Around line 160-167: The acceptChange() logic in ExprPacketField incorrectly
returns null for array-backed fields because expected.equals(typeToAccept) is
always false when expected.isArray(); update acceptChange() in class
ExprPacketField so that if fieldAccessor.expectedType().isArray() it returns new
Class[]{ fieldAccessor.expectedType().getComponentType() } (allowing
element-type changes for arrays), otherwise return new Class[]{
fieldAccessor.expectedType() }; keep change() unchanged (it already handles
arrays), and ensure UPDATE_SIGN / "sign lines" fields become modifiable.
In
`@src/main/java/dev/threeadd/packeteventssk/element/general/field/ServerBoundPacketFieldRegistrar.java`:
- Around line 51-65: The constructor lambda for the UPDATE_SIGN registration
uses a mismatched key ("block position") that never matches the registered
position field keys ("location vector"/"location"); update the constructor in
PacketFieldRegistry.INSTANCE.builder(PacketType.Play.Client.UPDATE_SIGN,
WrapperPlayClientUpdateSign.class) to call values.getRequired with the same key
used in the requiredField for the position (e.g., "location" or "location
vector"), or alternatively change the requiredField keys to "block position" so
they match; ensure the two places (the requiredField for Vector.class and the
constructor creating new WrapperPlayClientUpdateSign) use the identical key
string.
---
Nitpick comments:
In
`@src/main/java/dev/threeadd/packeteventssk/element/entity/section/SecExprNewFakeEntity.java`:
- Around line 33-47: The static VALIDATORS map is repopulated in register()
without clearing old entries, which can leave stale keys; modify the register()
method to clear VALIDATORS at the start (e.g., call VALIDATORS.clear()) before
iterating FakeEntityFieldRegistry.INSTANCE.getAllSchemas() and rebuilding
validators with SimpleEntryValidator.builder(), so the map only contains current
FieldSchema keys and their corresponding EntryValidator instances.
In
`@src/main/java/dev/threeadd/packeteventssk/element/general/section/SecExprNewPacket.java`:
- Around line 29-44: The static VALIDATORS map is never cleared so repeated
calls to register() can leave stale entries; at the start of the
register(Registration reg) method clear VALIDATORS (e.g., VALIDATORS.clear())
before iterating PacketFieldRegistry.INSTANCE.getAllSchemas() so you rebuild
validators from scratch for each registration; update the register method that
references VALIDATORS, SimpleEntryValidator.builder(), and PacketFieldRegistry
to perform the clear first.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 060d9c9c-cc00-45a3-bbb1-5b26ed651b53
📒 Files selected for processing (35)
build.gradle.ktssrc/main/java/dev/threeadd/packeteventssk/api/general/packet/PacketTypeRegistry.javasrc/main/java/dev/threeadd/packeteventssk/api/simple/PlayerSkinManager.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/EntityElementRegistration.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/Types.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntitiesAll.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityEventValue.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityFromId.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityFromUuid.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprFakeEntityField.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprMetaField.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprVisibleFakeEntities.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/living/ExprFakeLivingEntityAttribute.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/entity/FakeBaseEntityFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/entity/FakeEntityFieldRegistry.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/entity/FakePlayerEntityFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/BaseMetaFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/DisplayMetaFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/InteractionMetaFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/ItemMetaFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/LivingMetaFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/MetaFieldRegistry.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/section/SecExprNewFakeEntity.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/section/SecExprNewMeta.javasrc/main/java/dev/threeadd/packeteventssk/element/general/GeneralElementRegistration.javasrc/main/java/dev/threeadd/packeteventssk/element/general/Types.javasrc/main/java/dev/threeadd/packeteventssk/element/general/condition/CondPacketTypeIsBound.javasrc/main/java/dev/threeadd/packeteventssk/element/general/expression/prop/ExprMetaField.javasrc/main/java/dev/threeadd/packeteventssk/element/general/expression/prop/ExprPacketField.javasrc/main/java/dev/threeadd/packeteventssk/element/general/field/ClientBoundPacketFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/general/field/PacketFieldRegistry.javasrc/main/java/dev/threeadd/packeteventssk/element/general/field/ServerBoundPacketFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/general/field/SkBeePacketFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/general/section/SecExprNewPacket.javasrc/main/resources/config.yml
💤 Files with no reviewable changes (3)
- src/main/java/dev/threeadd/packeteventssk/api/simple/PlayerSkinManager.java
- src/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityEventValue.java
- src/main/java/dev/threeadd/packeteventssk/element/general/expression/prop/ExprMetaField.java
|
CodeRabbit: No |
Owner
Author
|
real |
…copilot and coderabbit noticed
Owner
Author
|
@CodeRabbit re-review |
|
(ᵔ ᴥ ᵔ)🐇 ✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 34 out of 35 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/DisplayMetaFieldRegistrar.java:84
- The setters for "display shadow radius" and "display shadow strength" look swapped: radius is currently writing via setShadowStrength(...), while strength writes via setShadowRadius(...). This will cause scripts setting radius/strength to affect the wrong underlying value.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/living/ExprFakeLivingEntityAttribute.java (1)
71-79:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix ADD/REMOVE to use attribute default instead of
0dwhen no property is present
getAttributeValue()returnspeAttr.getDefaultValue()when the attribute property is missing, butchange()seedscurrentValuewith0d(orElse(0d)), soADD/REMOVEcan diverge from the values exposed by reads for non-zero defaults.Suggested fix
double currentValue = livingFake.getAttributes().getProperties().stream() .filter(prop -> prop.getAttribute() == peAttr) .findFirst() .map(Property::getValue) - .orElse(0d); + .orElse(peAttr.getDefaultValue());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/living/ExprFakeLivingEntityAttribute.java` around lines 71 - 79, getAttributeValue(...) returns peAttr.getDefaultValue() when a property is missing, but change() seeds currentValue with 0d (see orElse(0d)), causing ADD/REMOVE to diverge from reads; update the change() logic to use the attribute's default (peAttr.getDefaultValue()) as the fallback/current baseline instead of 0d (or call getAttributeValue(...) to obtain the current baseline) so ADD/REMOVE compute deltas consistently with getAttributeValue; adjust references inside change() where currentValue is initialized (and any orElse(0d) usage) to use the default from peAttr or the getAttributeValue result.src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/DisplayMetaFieldRegistrar.java (1)
77-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix swapped shadow radius/strength wiring (and stop truncating to ints)
display shadow radiusis currently wired tosetShadowStrength, anddisplay shadow strengthis wired tosetShadowRadius(lines 77-84).- Constructor-side values use
intValue()for both properties, butAbstractDisplayMeta#setShadowRadius/setShadowStrengthtake floats (lines 302-309).Suggested fix
.optionalField(Number.class, AbstractDisplayMeta::getShadowRadius, - (meta, newNum) -> meta.setShadowStrength(newNum.floatValue()), + (meta, newNum) -> meta.setShadowRadius(newNum.floatValue()), "display shadow radius") .optionalField(Number.class, AbstractDisplayMeta::getShadowStrength, - (meta, newNum) -> meta.setShadowRadius(newNum.floatValue()), + (meta, newNum) -> meta.setShadowStrength(newNum.floatValue()), "display shadow strength") @@ Number shadowRadius = context.getOptional("display shadow radius", Number.class); if (shadowRadius != null) { - meta.setShadowRadius(shadowRadius.intValue()); + meta.setShadowRadius(shadowRadius.floatValue()); } Number shadowStrength = context.getOptional("display shadow strength", Number.class); if (shadowStrength != null) { - meta.setShadowStrength(shadowStrength.intValue()); + meta.setShadowStrength(shadowStrength.floatValue()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/DisplayMetaFieldRegistrar.java` around lines 77 - 84, The two optionalField registrations in DisplayMetaFieldRegistrar are swapped and use integer conversion; change the first to map AbstractDisplayMeta::getShadowRadius to meta.setShadowRadius(...) and the second to map AbstractDisplayMeta::getShadowStrength to meta.setShadowStrength(...), and ensure both setters receive newNum.floatValue() (not intValue()) to match AbstractDisplayMeta#setShadowRadius and `#setShadowStrength` which expect floats.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@src/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/living/ExprFakeLivingEntityAttribute.java`:
- Around line 71-79: getAttributeValue(...) returns peAttr.getDefaultValue()
when a property is missing, but change() seeds currentValue with 0d (see
orElse(0d)), causing ADD/REMOVE to diverge from reads; update the change() logic
to use the attribute's default (peAttr.getDefaultValue()) as the
fallback/current baseline instead of 0d (or call getAttributeValue(...) to
obtain the current baseline) so ADD/REMOVE compute deltas consistently with
getAttributeValue; adjust references inside change() where currentValue is
initialized (and any orElse(0d) usage) to use the default from peAttr or the
getAttributeValue result.
In
`@src/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/DisplayMetaFieldRegistrar.java`:
- Around line 77-84: The two optionalField registrations in
DisplayMetaFieldRegistrar are swapped and use integer conversion; change the
first to map AbstractDisplayMeta::getShadowRadius to meta.setShadowRadius(...)
and the second to map AbstractDisplayMeta::getShadowStrength to
meta.setShadowStrength(...), and ensure both setters receive newNum.floatValue()
(not intValue()) to match AbstractDisplayMeta#setShadowRadius and
`#setShadowStrength` which expect floats.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 27f7a44e-ba2f-478d-93c2-fe9670bb211e
📒 Files selected for processing (11)
src/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityFromUuid.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprFakeEntityField.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/ExprMetaField.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/expression/prop/living/ExprFakeLivingEntityAttribute.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/entity/FakePlayerEntityFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/DisplayMetaFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/entity/field/meta/LivingMetaFieldRegistrar.javasrc/main/java/dev/threeadd/packeteventssk/element/general/GeneralElementRegistration.javasrc/main/java/dev/threeadd/packeteventssk/element/general/Types.javasrc/main/java/dev/threeadd/packeteventssk/element/general/expression/prop/ExprPacketField.javasrc/main/java/dev/threeadd/packeteventssk/element/general/field/ServerBoundPacketFieldRegistrar.java
💤 Files with no reviewable changes (1)
- src/main/java/dev/threeadd/packeteventssk/element/general/GeneralElementRegistration.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/dev/threeadd/packeteventssk/element/entity/expression/ExprFakeEntityFromUuid.java
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.
Summary by CodeRabbit
New Features
Improvements
Updates
Removed