feat: add multiple entity instance entries #391
Conversation
WalkthroughAdds an abstract MultipleEntityDisplayBase and three concrete multi-audience entity displays (shared, individual, group), three new Multiple*AdvancedEntityInstanceEntry classes that create those displays, and a small import change replacing a specific import with a wildcard. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Viewer as Player/Viewer
participant Display as MultipleSharedAudienceEntityDisplay
participant Manager as SharedActivityManager[index]
participant Entity as DisplayEntity[index]
rect rgb(240,248,255)
Note over Display: initialize shared per-index managers
Display->>Manager: create manager[i]
end
loop Tick
Viewer->>Display: distance check
alt index i within showRange
Display->>Entity: spawn/ensure entity[i]
Display->>Manager: tick manager[i]
Manager-->>Display: state[i]
Display->>Entity: apply state[i]
else index i out of range
Display-->>Entity: despawn entity[i]
end
end
Note over Display: dispose
Display-->>Manager: dispose all
Display-->>Entity: remove all
sequenceDiagram
autonumber
participant Viewer as Player/Viewer
participant Display as MultipleIndividualAudienceEntityDisplay
participant Manager as ActivityManager[player,index]
participant Entity as DisplayEntity[player,index]
rect rgb(245,250,240)
Note over Display: on player add — create per-player per-index managers
Display->>Manager: create manager[player,i]
end
loop Tick (per player)
Viewer->>Display: per-player distance/multiplier check
alt index i within player showRange
Display->>Entity: spawn/ensure (player,i)
Display->>Manager: tick (player,i)
Manager-->>Display: state (player,i)
Display->>Entity: apply state
else out of range
Display-->>Entity: despawn (player,i)
end
end
Note over Display: player removal
Display-->>Manager: dispose (player, *)
Display-->>Entity: remove (player, *)
sequenceDiagram
autonumber
participant Viewer as Player/Viewer
participant Display as MultipleGroupAudienceEntityDisplay
participant GroupMgr as ActivityManager[group,index]
participant Entity as DisplayEntity[group,index]
rect rgb(255,250,240)
Note over Display: map players -> groups and create per-group managers
Display->>GroupMgr: create manager[group,i]
end
loop Tick (per group)
Viewer->>Display: group membership & distance check
alt index i within group's showRange
Display->>Entity: spawn/ensure (group,i)
Display->>GroupMgr: tick (group,i)
GroupMgr-->>Display: groupState[i]
Display->>Entity: apply groupState[i] for group viewers
else out of range
Display-->>Entity: despawn (group,i)
end
end
Note over Display: group or viewer removal
Display-->>GroupMgr: dispose (group, *)
Display-->>Entity: remove (group, *)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt (1)
12-12: Avoid wildcard import for java.util.Prefer importing only Optional to keep imports explicit and avoid collisions.
-import java.util.* +import java.util.Optionalextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (2)
196-201: Call removedViewer on player removal (symmetry with filter removal).When a player leaves, you dispose entities but do not notify
ActivityManager. MirroronPlayerFilterRemovedto prevent stale viewer state.override fun onPlayerRemove(player: Player) { - super.onPlayerRemove(player) - repeat(multiplier) { index -> - entities.remove(MultiKey(player.uniqueId, index))?.dispose() - } + super.onPlayerRemove(player) + repeat(multiplier) { index -> + val key = MultiKey(player.uniqueId, index) + entities.remove(key)?.dispose() + activityManagers[index]?.removedViewer(SharedActivityContext(instanceEntryRef, players), player) + } }
29-34: Add @Help for showRange and validate multiplier in editor.Improve editor UX and guard inputs.
- override val showRange: Optional<Var<Double>> = Optional.empty(), + @Help("Max distance (blocks) at which entities spawn/render for the player") + override val showRange: Optional<Var<Double>> = Optional.empty(), @@ - @Help("Number of entities to spawn per player") + @Help("Number of entities to spawn per player (must be >= 1)")Also applies to: 67-72
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (1)
187-194: Symmetric viewer cleanup on player removal (shared variant).On filter removal you notify managers, but on player removal you don’t. Add
removedViewerto avoid stale viewer state.override fun onPlayerRemove(player: Player) { super.onPlayerRemove(player) repeat(multiplier) { index -> - entities.remove(MultiKey(player.uniqueId, index))?.dispose() + val key = MultiKey(player.uniqueId, index) + entities.remove(key)?.dispose() + activityManagers[index]?.removedViewer(SharedActivityContext(instanceEntryRef, players), player) } }Also applies to: 196-201
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt
📄 CodeRabbit inference engine (.github/instructions/entity-extension.instructions.md)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt: Annotate all entry classes with @entry(id, description, Colors, icon); use @tags on definition entries; use @OnlyTags on data lists to constrain allowed data types
Use snake_case, descriptive IDs (e.g., armor_stand_definition, allay_instance) and practical descriptions
Use icons from established packs (lucide:, ph:, fa6-solid:, etc.) and colors from com.typewritermc.core.books.pages.Colors
Apply data tagging: use "generic_entity_data" for generic properties, "living_entity_data" for living-specific, and add per-entity tag (e.g., "armor_stand_data"); reference them via @OnlyTags on definitions/instances
Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var, sound: Var, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Runtime wrapper entities should extend WrapperFakeEntity(EntityTypes., player)
Override applyProperty in wrapper to route per-entity properties first, then fall back to applyGenericEntityData and applyLivingEntityData
Do not manually manage viewers or metadata batching in wrappers; rely on WrapperFakeEntity (spawn, viewers, passengers, setNotifyAboutChanges)
For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Create a matching entry class XData(...) : GenericEntityData implementing type() and build(player)
Implement appliers: fun applyXData(entity: WrapperEntity, property: XProperty) using entity.metas { meta<...> { ... } } with a helpful error fallback
Hook per-...
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt
📄 CodeRabbit inference engine (engine/AGENTS.md)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt: Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin source files
Document public APIs with KDoc, focusing on when to use them rather than method-by-method descriptions
Avoid inline comments; refactor unclear code instead of adding explanations
Files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
🧠 Learnings (13)
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Use SimpleEntityDefinition + SimpleEntityInstance for straightforward cases; use Shared/Group/Individual AdvancedEntityInstance entries for complex lifecycles/show ranges
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Instance class <EntityName>Instance must be annotated with Entry and declare fields: id, name, definition: Ref<<EntityName>Definition>, spawnLocation, data (+OnlyTags as above), activity: Ref<out SharedEntityActivityEntry>
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Avoid mutating Bukkit entities directly; operate via WrapperEntity and Typewriter EntityProperty APIs
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Activities: implement GenericEntityActivityEntry and return an EntityActivity via create(context, currentLocation)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Keep visual and structural parity with similar entities (icons, colors, descriptions, class structure) like AllayEntity.kt or ArmorStandEntity.kt
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Entry IDs for both definition and instance must be snake_case and unique (e.g., allay_definition, allay_instance)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Create a new vanilla entity file at entries/entity/minecraft/<EntityName>Entity.kt that defines <EntityName>Definition, <EntityName>Instance, and a private <EntityName>Entity wrapper
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Create a matching entry class XData(...) : GenericEntityData<XProperty> implementing type() and build(player)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
🧬 Code graph analysis (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/EntryReference.kt (1)
emptyRef(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (2)
241-256: Filtering before managers exist (individual variant) — confirm pipeline.If
filter()runs beforeonPlayerAdd,getMultiplierCountis 0 and this always returns false. EnsureonPlayerAddprecedes filtering or makefilter()handle the pre-init case.See companion comment in entries file for options; verify behavior in AudienceFilter.
214-224: Reasonable fallbacks for state/position — LGTM.Using the closest entity when available and falling back to index 0 state/position is pragmatic.
If there’s a non-zero default index other than 0 in your managers, confirm alignment.
Also applies to: 365-371
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
245-256: Incorrect target file for lifecycle verification
The fileMultipleAdvancedEntityInstanceEntry.ktcontains onlydisplay()and helper methods—nofilter()oronPlayerAdd()overrides exist here. The lifecycle concern aroundactivityManagersandgetMultiplierCountapplies toMultipleAudienceEntityDisplay(inengine-paper), not this entry class. Please update the review to point at the correct class or confirm which component needs lifecycle validation.Likely an incorrect or invalid review comment.
.../main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
My main thing which I would like to have changed is that instead of having this MultiKey map thing, I would argue that having a ConcurrentMap<PlayerID, List<SubObjects>> (like activity managers, displays, entity data) would be better.
We never have any holes in the list, it makes it easier to access. Instead of you doing this (0 until count) stuff you can just get the list. The list itself would be immutable, to change it you would just compute a new value on the map (which is thread safe).
This would likely make make much of the code much more readable and likely won't have a big performance impact, as we almost always list through things anyway and rarely access a random index anyway.
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
steveb05
force-pushed
the
feat/multiple-advanced-entity
branch
from
d361225 to
3cc3ad1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
115-138: Verify type compatibility between EntityData and PropertySupplier.A past review flagged that
BaseInfo.suppliersexpectsList<Pair<PropertySupplier<*>, Int>>butgetBaseInfoappears to passEntityDatainstances directly. Verify that:
definition.data.withPriority()returnsList<Pair<PropertySupplier<*>, Int>>(notEntityData)- If it returns
EntityData, you need an explicit conversion (e.g.,.map { (data, priority) -> data.toSupplier() to priority })Run this script to check the return type of
withPriority()and whether conversion is needed:#!/bin/bash # Check withPriority return type and EntityData/PropertySupplier relationship rg -nP 'fun\s+.*withPriority\s*\(' -A5 # Look for PropertySupplier type definition rg -nP 'typealias\s+PropertySupplier|interface\s+PropertySupplier|class\s+PropertySupplier' -C3 # Check if EntityData can be used as PropertySupplier rg -nP 'interface\s+EntityData|class\s+EntityData' -A10 | rg -P 'PropertySupplier|toSupplier'
🧹 Nitpick comments (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (3)
18-18: Simplify to regular HashMap for entities storage.Based on past feedback,
ConcurrentHashMapis unnecessary here. Entity state corrections happen next tick, so transient inconsistencies are acceptable. A regularHashMapormutableMapOf()is sufficient and simpler.Apply this diff:
- protected val entities = ConcurrentHashMap<UUID, List<DisplayEntity>>() + protected val entities = mutableMapOf<UUID, List<DisplayEntity>>()
111-111: Consider making entity count dynamic in shared display.Per past feedback, the count should be dynamic—allowing NPCs to be added/removed when it changes. Currently,
entityCountis fixed at initialization. Consider fetching it on each tick and scaling managers up/down, similar to the individual display's implementation (lines 304-345).To make this dynamic, you would:
- Accept
entityCountas aVar<Int>instead ofInt- In
tick(), compare current count withactivityManagers.size- Scale up (create new managers) or down (dispose excess managers) as needed
248-249: Replace ConcurrentHashMap with regular collections.Based on past feedback, these concurrent maps are unnecessary:
activityManagers: Created once per player, removed only in dispose—use a regularmutableMapOf()lastStates: State inconsistencies self-correct next tick—use a regularmutableMapOf()Apply this diff:
- private val activityManagers = ConcurrentHashMap<UUID, List<ActivityManager<in IndividualActivityContext>>>() - private val lastStates = ConcurrentHashMap<UUID, MutableList<EntityState>>() + private val activityManagers = mutableMapOf<UUID, List<ActivityManager<in IndividualActivityContext>>>() + private val lastStates = mutableMapOf<UUID, MutableList<EntityState>>()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
🧰 Additional context used
📓 Path-based instructions (5)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt
📄 CodeRabbit inference engine (engine/AGENTS.md)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt: Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin source files
Document public APIs with KDoc, focusing on when to use them rather than method-by-method descriptions
Avoid inline comments; refactor unclear code instead of adding explanations
Files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt
📄 CodeRabbit inference engine (.github/instructions/entity-extension.instructions.md)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt: Annotate all entry classes with @entry(id, description, Colors, icon); use @tags on definition entries; use @OnlyTags on data lists to constrain allowed data types
Use snake_case, descriptive IDs (e.g., armor_stand_definition, allay_instance) and practical descriptions
Use icons from established packs (lucide:, ph:, fa6-solid:, etc.) and colors from com.typewritermc.core.books.pages.Colors
Apply data tagging: use "generic_entity_data" for generic properties, "living_entity_data" for living-specific, and add per-entity tag (e.g., "armor_stand_data"); reference them via @OnlyTags on definitions/instances
Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var, sound: Var, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Runtime wrapper entities should extend WrapperFakeEntity(EntityTypes., player)
Override applyProperty in wrapper to route per-entity properties first, then fall back to applyGenericEntityData and applyLivingEntityData
Do not manually manage viewers or metadata batching in wrappers; rely on WrapperFakeEntity (spawn, viewers, passengers, setNotifyAboutChanges)
For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Create a matching entry class XData(...) : GenericEntityData implementing type() and build(player)
Implement appliers: fun applyXData(entity: WrapperEntity, property: XProperty) using entity.metas { meta<...> { ... } } with a helpful error fallback
Hook per-...
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Use SimpleEntityDefinition + SimpleEntityInstance for straightforward cases; use Shared/Group/Individual AdvancedEntityInstance entries for complex lifecycles/show ranges
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Instance class <EntityName>Instance must be annotated with Entry and declare fields: id, name, definition: Ref<<EntityName>Definition>, spawnLocation, data (+OnlyTags as above), activity: Ref<out SharedEntityActivityEntry>
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Create a new vanilla entity file at entries/entity/minecraft/<EntityName>Entity.kt that defines <EntityName>Definition, <EntityName>Instance, and a private <EntityName>Entity wrapper
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Activities: implement GenericEntityActivityEntry and return an EntityActivity via create(context, currentLocation)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Create a matching entry class XData(...) : GenericEntityData<XProperty> implementing type() and build(player)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Entry IDs for both definition and instance must be snake_case and unique (e.g., allay_definition, allay_instance)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
🧬 Code graph analysis (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/EntryReference.kt (1)
emptyRef(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (3)
101-102: Good: Immutable list with var for activityManagers.This addresses the past feedback correctly by using a
varwith an immutableListthat gets reassigned, rather than a mutable collection.
304-345: Excellent: Dynamic count adjustment implemented.This correctly implements the past feedback to make count dynamic. The tick method compares current count with manager size and scales up (creating new managers) or down (disposing excess managers) as needed. This enables truly dynamic NPC spawning.
261-261: Fix boundary condition: should becount <= 0.Per past feedback, filtering should reject zero or negative counts.
Apply this diff:
- if (count <= 0) return false + if (count <= 0) return falseWait, the code already has
count <= 0. Let me re-check... The existing code at line 261 is correct. This appears to already address the past feedback.extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (2)
19-46: LGTM: Entry annotation and structure are correct.The entry follows coding guidelines:
- Properly annotated with
@Entryincluding snake_case ID, description, color, and icon- Includes helpful KDoc explaining usage
- Uses
@Help,@Min, and@Defaultannotations for better UX- Implements the correct interface (
SharedAdvancedEntityInstance)- Activity scope is correct (Shared for shared behavior)
67-94: LGTM: Entry annotation and structure are correct.The entry follows coding guidelines:
- Properly annotated with snake_case ID and metadata
- Includes helpful KDoc with use case
- Correct interface implementation (
IndividualAdvancedEntityInstance)- Correct activity scope (Individual for per-viewer behavior)
- Uses
Var<Int>for dynamic count, enabling runtime changesBased on learnings
steveb05
force-pushed
the
feat/multiple-advanced-entity
branch
2 times, most recently
from
3db1fae to
1ebb7c4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
129-137: Cast EntityData to PropertySupplier in overrideSuppliersoverrideSuppliers builds List<Pair<EntityData<>, Int>> but BaseInfo.suppliers is List<Pair<PropertySupplier<>, Int>>. Since EntityData implements PropertySupplier, cast each data element:
- val overrideSuppliers = children + val overrideSuppliers: List<Pair<PropertySupplier<*>, Int>> = children .descendants(EntityData::class) .mapNotNull { it.get() } - .map { data -> data to (data.priority + maxBasePriority + 1) } + .map { data -> (data as PropertySupplier<*>) to (data.priority + maxBasePriority + 1) }
🧹 Nitpick comments (2)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (2)
81-94: Consider runtime clamping for Var countYou have @min(1), but a defensive clamp avoids surprises if values come from scripts or vars. Optionally wrap the var (if supported) and pass the clamped value to the display.
For example (adjust to your Var API):
- return MultipleIndividualAudienceEntityDisplay( + // Optional: clamp to >= 1 at runtime + val safeCount = count /* e.g., count.map { it.coerceAtLeast(1) } */ + return MultipleIndividualAudienceEntityDisplay( ref(), definition, activityCreator, suppliers, spawnLocation, showRange.orElse(ConstVar(entityShowRange)), - count, + safeCount, )As per coding guidelines
33-46: Optional: add an upper bound to count to prevent accidental overloadConsider adding a @max (if available) or a guard to keep count within sane limits to avoid server churn from massive spawns.
Also applies to: 81-94
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
🧰 Additional context used
📓 Path-based instructions (4)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt
📄 CodeRabbit inference engine (.github/instructions/entity-extension.instructions.md)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt: Annotate all entry classes with @entry(id, description, Colors, icon); use @tags on definition entries; use @OnlyTags on data lists to constrain allowed data types
Use snake_case, descriptive IDs (e.g., armor_stand_definition, allay_instance) and practical descriptions
Use icons from established packs (lucide:, ph:, fa6-solid:, etc.) and colors from com.typewritermc.core.books.pages.Colors
Apply data tagging: use "generic_entity_data" for generic properties, "living_entity_data" for living-specific, and add per-entity tag (e.g., "armor_stand_data"); reference them via @OnlyTags on definitions/instances
Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var, sound: Var, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Runtime wrapper entities should extend WrapperFakeEntity(EntityTypes., player)
Override applyProperty in wrapper to route per-entity properties first, then fall back to applyGenericEntityData and applyLivingEntityData
Do not manually manage viewers or metadata batching in wrappers; rely on WrapperFakeEntity (spawn, viewers, passengers, setNotifyAboutChanges)
For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Create a matching entry class XData(...) : GenericEntityData implementing type() and build(player)
Implement appliers: fun applyXData(entity: WrapperEntity, property: XProperty) using entity.metas { meta<...> { ... } } with a helpful error fallback
Hook per-...
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Use SimpleEntityDefinition + SimpleEntityInstance for straightforward cases; use Shared/Group/Individual AdvancedEntityInstance entries for complex lifecycles/show ranges
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Instance class <EntityName>Instance must be annotated with Entry and declare fields: id, name, definition: Ref<<EntityName>Definition>, spawnLocation, data (+OnlyTags as above), activity: Ref<out SharedEntityActivityEntry>
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Avoid mutating Bukkit entities directly; operate via WrapperEntity and Typewriter EntityProperty APIs
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Activities: implement GenericEntityActivityEntry and return an EntityActivity via create(context, currentLocation)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Keep visual and structural parity with similar entities (icons, colors, descriptions, class structure) like AllayEntity.kt or ArmorStandEntity.kt
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Entry IDs for both definition and instance must be snake_case and unique (e.g., allay_definition, allay_instance)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Create a new vanilla entity file at entries/entity/minecraft/<EntityName>Entity.kt that defines <EntityName>Definition, <EntityName>Instance, and a private <EntityName>Entity wrapper
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
🧬 Code graph analysis (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/EntryReference.kt (1)
emptyRef(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt (1)
12-12: Wildcard import is fine hereAcceptable to bring in Optional/empty without extra noise. No functional impact.
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (3)
19-46: Entry metadata and scopes look correctIDs are snake_case, colors/icons consistent, and activity scopes match entry types.
As per coding guidelines
48-61: Display creation for shared variant looks solidGood: fallback to IdleActivity, default showRange, and clamping count ≥ 1.
115-118: BaseInfo.suppliers type matches display expectationsUsing List<Pair<PropertySupplier<*>, Int>> aligns with collectors and display plumbing.
steveb05
force-pushed
the
feat/multiple-advanced-entity
branch
from
1ebb7c4 to
a1c8f37
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
180-189: Convert EntityData to PropertySupplier before combining (compile-time mismatch).
BaseInfo.suppliersrequiresList<Pair<PropertySupplier<*>, Int>>, butoverrideSuppliersareEntityData. Convert via your existing helper (e.g.,toSupplier()).- val baseSuppliers = definition.data.withPriority() + val baseSuppliers: List<Pair<PropertySupplier<*>, Int>> = definition.data.withPriority() @@ - val overrideSuppliers = children + val overrideSuppliers: List<Pair<PropertySupplier<*>, Int>> = children .descendants(EntityData::class) .mapNotNull { it.get() } - .map { data -> data to (data.priority + maxBasePriority + 1) } + .map { data -> + // Replace `toSupplier()` with the correct converter in your codebase. + data.toSupplier() to (data.priority + maxBasePriority + 1) + } - return BaseInfo(definition, baseSuppliers + overrideSuppliers) + return BaseInfo(definition, baseSuppliers + overrideSuppliers)If your converter is named differently (e.g.,
supplier(),asSupplier()), adjust accordingly.
🧹 Nitpick comments (6)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt (1)
12-12: Prefer explicit Optional import overjava.util.*.Avoid wildcard imports to reduce namespace leakage; import
java.util.Optionaldirectly.Apply:
-import java.util.* +import java.util.Optionalengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (3)
254-256: ConcurrentHashMap likely unnecessary; prefer regular mutable maps.All mutations occur on the main tick thread; thread-safety isn’t required here. Using
mutableMapOf()reduces overhead and clarifies single-threaded assumptions.- private val activityManagers = ConcurrentHashMap<UUID, List<ActivityManager<in IndividualActivityContext>>>() - private val lastStates = ConcurrentHashMap<UUID, MutableList<EntityState>>() + private val activityManagers = mutableMapOf<UUID, List<ActivityManager<in IndividualActivityContext>>>() + private val lastStates = mutableMapOf<UUID, MutableList<EntityState>>()
461-463: Scope group viewers to considered players.
players.filter { ... }may include non-considered players. UseconsideredPlayersto align with display lifecycle and avoid spurious viewers.- return players.filter { group.groupId(it) == groupId } + return consideredPlayers.filter { group.groupId(it) == groupId }
115-121: Initialize with empty viewers; ensure contexts include the newly added player.
viewingPlayersis derived from spawned entities; duringinitializeit’s empty, and inonPlayerFilterAddedtheaddedViewercall uses a viewers list that may not yet include the current player. Consider passing a viewers list that includes the current player for that call.- createDisplayEntity(player, manager).also { - manager.addedViewer(SharedActivityContext(instanceEntryRef, viewingPlayers), player) - } + createDisplayEntity(player, manager).also { + val viewers = (viewingPlayers + player).distinct() + manager.addedViewer(SharedActivityContext(instanceEntryRef, viewers), player) + }extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (2)
33-61: Shared multiple count is static; consider making it dynamic like Individual.For parity and usefulness, consider
count: Var<Int>so changes adjust spawned instances at runtime (as you already do in Individual).- @Default("1") - val count: Int = 1, + @Default("1") + val count: Var<Int> = ConstVar(1), @@ - count.coerceAtLeast(1), + count.getOrElse(ConstVar(1)).coerceAtLeast(1),And update the corresponding display to accept
Var<Int>and scale intick().
129-160: Group multiple count is static; consider making it dynamic like Individual.Making
countaVar<Int>enables live scaling per group without reloading.- @Default("1") - val count: Int = 1, + @Default("1") + val count: Var<Int> = ConstVar(1), @@ - count.coerceAtLeast(1), + count.getOrElse(ConstVar(1)).coerceAtLeast(1),You’ll also need to make the group display scale its manager/entity lists when the value changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt
📄 CodeRabbit inference engine (.github/instructions/entity-extension.instructions.md)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt: Annotate all entry classes with @entry(id, description, Colors, icon); use @tags on definition entries; use @OnlyTags on data lists to constrain allowed data types
Use snake_case, descriptive IDs (e.g., armor_stand_definition, allay_instance) and practical descriptions
Use icons from established packs (lucide:, ph:, fa6-solid:, etc.) and colors from com.typewritermc.core.books.pages.Colors
Apply data tagging: use "generic_entity_data" for generic properties, "living_entity_data" for living-specific, and add per-entity tag (e.g., "armor_stand_data"); reference them via @OnlyTags on definitions/instances
Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var, sound: Var, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Runtime wrapper entities should extend WrapperFakeEntity(EntityTypes., player)
Override applyProperty in wrapper to route per-entity properties first, then fall back to applyGenericEntityData and applyLivingEntityData
Do not manually manage viewers or metadata batching in wrappers; rely on WrapperFakeEntity (spawn, viewers, passengers, setNotifyAboutChanges)
For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Create a matching entry class XData(...) : GenericEntityData implementing type() and build(player)
Implement appliers: fun applyXData(entity: WrapperEntity, property: XProperty) using entity.metas { meta<...> { ... } } with a helpful error fallback
Hook per-...
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt
📄 CodeRabbit inference engine (engine/AGENTS.md)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt: Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin source files
Document public APIs with KDoc, focusing on when to use them rather than method-by-method descriptions
Avoid inline comments; refactor unclear code instead of adding explanations
Files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
🧠 Learnings (16)
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Use SimpleEntityDefinition + SimpleEntityInstance for straightforward cases; use Shared/Group/Individual AdvancedEntityInstance entries for complex lifecycles/show ranges
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Instance class <EntityName>Instance must be annotated with Entry and declare fields: id, name, definition: Ref<<EntityName>Definition>, spawnLocation, data (+OnlyTags as above), activity: Ref<out SharedEntityActivityEntry>
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Activities: implement GenericEntityActivityEntry and return an EntityActivity via create(context, currentLocation)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Create a new vanilla entity file at entries/entity/minecraft/<EntityName>Entity.kt that defines <EntityName>Definition, <EntityName>Instance, and a private <EntityName>Entity wrapper
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Keep visual and structural parity with similar entities (icons, colors, descriptions, class structure) like AllayEntity.kt or ArmorStandEntity.kt
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Create a matching entry class XData(...) : GenericEntityData<XProperty> implementing type() and build(player)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var<String>, sound: Var<Sound>, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Avoid mutating Bukkit entities directly; operate via WrapperEntity and Typewriter EntityProperty APIs
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Entry IDs for both definition and instance must be snake_case and unique (e.g., allay_definition, allay_instance)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : InteractEntityObjective text may include <entity> placeholder; obtain positions by querying active AudienceEntityDisplay instances
Applied to files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
🧬 Code graph analysis (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/EntryReference.kt (1)
emptyRef(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (1)
265-273: Remove the proposed spawn‐position fallback—override onPlayerAdd populatesactivityManagersbefore callingsuper.onPlayerAdd, which invokesfilter, somanagers()is never empty at filter execution.Likely an incorrect or invalid review comment.
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
steveb05
force-pushed
the
feat/multiple-advanced-entity
branch
2 times, most recently
from
649946e to
779be19
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (2)
492-500: Group filter gates initialization; add spawnPosition fallback when managers are absent.When no managers exist yet, filter returns false, blocking first viewer initialization.
override fun filter(player: Player): Boolean { val groupId = group.groupId(player) ?: GroupId(player.uniqueId) - val managerList = activityManagers[groupId] ?: return false - val showRangeSq = showRangeSq(player) - - return managerList.any { manager -> + val managerList = activityManagers[groupId] + val showRangeSq = showRangeSq(player) + + if (managerList.isNullOrEmpty()) { + return spawnPosition.distanceSqrt(player.location)?.let { it <= showRangeSq } ?: false + } + return managerList.any { manager -> manager.position.distanceSqrt(player.location)?.let { it <= showRangeSq } ?: false } }
568-596: Group: defer addedViewer until after entities update; compute per-manager viewers.Mirror the Shared logic to avoid missing the current player in viewers and to scope viewers correctly.
- val newEntities = managerList.mapNotNull { manager -> + val managersToAdd = mutableListOf<ActivityManager<*>>() + val newEntities = managerList.mapNotNull { manager -> val distance = manager.position.distanceSqrt(player.location) val currentEntity = currentEntities.find { it.activityManager === manager } when { distance == null || distance > showRangeSq -> { currentEntity?.dispose() manager.removedViewer(SharedActivityContext(instanceEntryRef, groupViewers(groupId)), player) null } currentEntity != null -> { currentEntity } else -> { - createDisplayEntity(player, manager).also { - manager.addedViewer(SharedActivityContext(instanceEntryRef, groupViewers(groupId)), player) - } + managersToAdd.add(manager) + createDisplayEntity(player, manager) } } } if (newEntities.isNotEmpty()) { entities[player.uniqueId] = newEntities + if (managersToAdd.isNotEmpty()) { + managersToAdd.forEach { m -> + val viewers = entities.entries.mapNotNull { (pid, list) -> + val p = server.getPlayer(pid) + if (p != null && group.groupId(p) == groupId && list.any { it.activityManager === m }) p else null + } + m.addedViewer(SharedActivityContext(instanceEntryRef, viewers), player) + } + } } else { entities.remove(player.uniqueId) }
🧹 Nitpick comments (1)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (1)
271-273: Prefer mutableMap over ConcurrentHashMap (main-threaded).If all mutations occur on the server main thread (typical for Paper), drop CHM to reduce overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
🧰 Additional context used
📓 Path-based instructions (5)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt
📄 CodeRabbit inference engine (.github/instructions/entity-extension.instructions.md)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt: Annotate all entry classes with @entry(id, description, Colors, icon); use @tags on definition entries; use @OnlyTags on data lists to constrain allowed data types
Use snake_case, descriptive IDs (e.g., armor_stand_definition, allay_instance) and practical descriptions
Use icons from established packs (lucide:, ph:, fa6-solid:, etc.) and colors from com.typewritermc.core.books.pages.Colors
Apply data tagging: use "generic_entity_data" for generic properties, "living_entity_data" for living-specific, and add per-entity tag (e.g., "armor_stand_data"); reference them via @OnlyTags on definitions/instances
Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var, sound: Var, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Runtime wrapper entities should extend WrapperFakeEntity(EntityTypes., player)
Override applyProperty in wrapper to route per-entity properties first, then fall back to applyGenericEntityData and applyLivingEntityData
Do not manually manage viewers or metadata batching in wrappers; rely on WrapperFakeEntity (spawn, viewers, passengers, setNotifyAboutChanges)
For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Create a matching entry class XData(...) : GenericEntityData implementing type() and build(player)
Implement appliers: fun applyXData(entity: WrapperEntity, property: XProperty) using entity.metas { meta<...> { ... } } with a helpful error fallback
Hook per-...
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt
📄 CodeRabbit inference engine (engine/AGENTS.md)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt: Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin source files
Document public APIs with KDoc, focusing on when to use them rather than method-by-method descriptions
Avoid inline comments; refactor unclear code instead of adding explanations
Files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
🧠 Learnings (14)
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Use SimpleEntityDefinition + SimpleEntityInstance for straightforward cases; use Shared/Group/Individual AdvancedEntityInstance entries for complex lifecycles/show ranges
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Instance class <EntityName>Instance must be annotated with Entry and declare fields: id, name, definition: Ref<<EntityName>Definition>, spawnLocation, data (+OnlyTags as above), activity: Ref<out SharedEntityActivityEntry>
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.ktengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Activities: implement GenericEntityActivityEntry and return an EntityActivity via create(context, currentLocation)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Create a new vanilla entity file at entries/entity/minecraft/<EntityName>Entity.kt that defines <EntityName>Definition, <EntityName>Instance, and a private <EntityName>Entity wrapper
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Keep visual and structural parity with similar entities (icons, colors, descriptions, class structure) like AllayEntity.kt or ArmorStandEntity.kt
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Use EntityLib metas via entity.metas { meta<...> { ... } } for entity-specific metadata and rely on shared appliers for generic/living
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var<String>, sound: Var<Sound>, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : InteractEntityObjective text may include <entity> placeholder; obtain positions by querying active AudienceEntityDisplay instances
Applied to files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
🧬 Code graph analysis (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/EntryReference.kt (1)
emptyRef(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
180-189: Remove unnecessary supplier conversion suggestion
EntityData implements PropertySupplierand withPriority() returns List<Pair<EntityData<>, Int>>; since Pair and List are covariant, that satisfies List<Pair<PropertySupplier<>, Int>> and compiles as-is—no toSupplier() helper exists.
Likely an incorrect or invalid review comment.
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Outdated
Show resolved
Hide resolved
steveb05
force-pushed
the
feat/multiple-advanced-entity
branch
from
779be19 to
2a92c54
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (4)
519-527: Group: optional fallback when managers aren’t ready.Consider allowing proximity to spawnPosition to pass filter until managers exist.
- val managerList = activityManagers[groupId] ?: return false + val managerList = activityManagers[groupId] val showRangeSq = showRangeSq(player) - return managerList.any { manager -> + if (managerList.isNullOrEmpty()) { + return spawnPosition.distanceSqrt(player.location)?.let { it <= showRangeSq } ?: false + } + return managerList.any { manager -> manager.position.distanceSqrt(player.location)?.let { it <= showRangeSq } ?: false }Please confirm AudienceFilter lifecycle: does onPlayerAdd run before filter? If yes, this fallback may be unnecessary.
274-276: Use simple mutable maps; CHM isn’t needed on main thread.Prefer mutableMapOf with vars for lists; simpler and cheaper.
- private val activityManagers = ConcurrentHashMap<UUID, List<ActivityManager<in IndividualActivityContext>>>() - private val lastStates = ConcurrentHashMap<UUID, MutableList<EntityState>>() + private val activityManagers = mutableMapOf<UUID, List<ActivityManager<in IndividualActivityContext>>>() + private val lastStates = mutableMapOf<UUID, MutableList<EntityState>>()- private val activityManagers = ConcurrentHashMap<GroupId, List<ActivityManager<in SharedActivityContext>>>() - private val lastStates = ConcurrentHashMap<GroupId, MutableList<EntityState>>() + private val activityManagers = mutableMapOf<GroupId, List<ActivityManager<in SharedActivityContext>>>() + private val lastStates = mutableMapOf<GroupId, MutableList<EntityState>>()Also applies to: 502-504
554-565: Group: addedViewer should use per-manager viewers and run after entities map update.Currently you notify with groupViewers (too broad) and before entities are stored. Align with Shared variant.
@@ - val newEntities = managerList.mapNotNull { manager -> - manager.position.distanceSqrt(player.location)?.takeIf { it <= showRangeSq }?.let { - createDisplayEntity(player, manager).also { - manager.addedViewer(SharedActivityContext(instanceEntryRef, groupViewers(groupId)), player) - } - } - } + val newEntities = managerList.mapNotNull { manager -> + manager.position.distanceSqrt(player.location)?.takeIf { it <= showRangeSq }?.let { + createDisplayEntity(player, manager) + } + } @@ - if (newEntities.isNotEmpty()) { - entities[player.uniqueId] = newEntities - } + if (newEntities.isNotEmpty()) { + entities[player.uniqueId] = newEntities + newEntities.forEach { entity -> + val viewers = entities.entries + .filter { (_, list) -> list.any { it.activityManager === entity.activityManager } } + .mapNotNull { (pid, _) -> server.getPlayer(pid) } + entity.activityManager.addedViewer(SharedActivityContext(instanceEntryRef, viewers), player) + } + }Also applies to: 621-626
316-323: Individual: use IndividualActivityContext (not Shared) and notify managers before dispose.Passing SharedActivityContext breaks semantics; also call removedViewer before disposing managers.
@@ - val newEntities = managerList.mapNotNull { manager -> - manager.position.distanceSqrt(player.location)?.takeIf { it <= showRangeSq }?.let { - createDisplayEntity(player, manager).also { - manager.addedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) - } - } - } + val newEntities = managerList.mapNotNull { manager -> + manager.position.distanceSqrt(player.location)?.takeIf { it <= showRangeSq }?.let { + createDisplayEntity(player, manager).also { + manager.addedViewer(IndividualActivityContext(instanceEntryRef, player), player) + } + } + } @@ - currentEntities.drop(currentCount).forEach { entity -> - entity.activityManager.removedViewer( - SharedActivityContext(instanceEntryRef, listOf(player)), - player - ) - entity.dispose() - } + currentEntities.drop(currentCount).forEach { entity -> + entity.activityManager.removedViewer(IndividualActivityContext(instanceEntryRef, player), player) + entity.dispose() + } @@ - distance == null || distance > showRangeSq -> { - currentEntity?.let { - manager.removedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) - it.dispose() - } + distance == null || distance > showRangeSq -> { + currentEntity?.let { + manager.removedViewer(IndividualActivityContext(instanceEntryRef, player), player) + it.dispose() + } null } @@ - else -> { - createDisplayEntity(player, manager).also { - manager.addedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) - } - } + else -> { + createDisplayEntity(player, manager).also { + manager.addedViewer(IndividualActivityContext(instanceEntryRef, player), player) + } + } @@ - entities[player.uniqueId]?.forEach { entity -> - entity.activityManager.removedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) - entity.dispose() - } + entities[player.uniqueId]?.forEach { entity -> + entity.activityManager.removedViewer(IndividualActivityContext(instanceEntryRef, player), player) + entity.dispose() + } @@ - entities[player.uniqueId]?.forEach { entity -> - entity.activityManager.removedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) - entity.dispose() - } + entities[player.uniqueId]?.forEach { entity -> + entity.activityManager.removedViewer(IndividualActivityContext(instanceEntryRef, player), player) + entity.dispose() + } entities.remove(player.uniqueId) - activityManagers.remove(player.uniqueId)?.forEach { manager -> - manager.dispose(IndividualActivityContext(instanceEntryRef, player)) - } + activityManagers.remove(player.uniqueId)?.forEach { manager -> + manager.removedViewer(IndividualActivityContext(instanceEntryRef, player), player) + manager.dispose(IndividualActivityContext(instanceEntryRef, player)) + }Also applies to: 369-379, 410-425, 441-447, 452-460
🧹 Nitpick comments (2)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt (1)
12-12: Avoid wildcard import; narrow to Optional only.Use a specific import to reduce namespace pollution and align with common Kotlin style.
-import java.util.* +import java.util.Optionalengine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (1)
597-618: Group: dynamic add/remove should also use per-manager viewers (not all group members).Notify only viewers that actually see this manager.
- distance == null || distance > showRangeSq -> { - currentEntity?.let { - manager.removedViewer( - SharedActivityContext(instanceEntryRef, groupViewers(groupId)), - player - ) - it.dispose() - } + distance == null || distance > showRangeSq -> { + currentEntity?.let { + val viewers = entities.entries + .filter { (_, list) -> list.any { it.activityManager === manager } } + .mapNotNull { (pid, _) -> server.getPlayer(pid) } + manager.removedViewer(SharedActivityContext(instanceEntryRef, viewers), player) + it.dispose() + } null } @@ - else -> { - createDisplayEntity(player, manager).also { - manager.addedViewer(SharedActivityContext(instanceEntryRef, groupViewers(groupId)), player) - } - } + else -> { + createDisplayEntity(player, manager).also { + val viewers = entities.entries + .filter { (_, list) -> list.any { it.activityManager === manager } } + .mapNotNull { (pid, _) -> server.getPlayer(pid) } + manager.addedViewer(SharedActivityContext(instanceEntryRef, viewers), player) + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt
📄 CodeRabbit inference engine (engine/AGENTS.md)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt: Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin source files
Document public APIs with KDoc, focusing on when to use them rather than method-by-method descriptions
Avoid inline comments; refactor unclear code instead of adding explanations
Files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt
📄 CodeRabbit inference engine (.github/instructions/entity-extension.instructions.md)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt: Annotate all entry classes with @entry(id, description, Colors, icon); use @tags on definition entries; use @OnlyTags on data lists to constrain allowed data types
Use snake_case, descriptive IDs (e.g., armor_stand_definition, allay_instance) and practical descriptions
Use icons from established packs (lucide:, ph:, fa6-solid:, etc.) and colors from com.typewritermc.core.books.pages.Colors
Apply data tagging: use "generic_entity_data" for generic properties, "living_entity_data" for living-specific, and add per-entity tag (e.g., "armor_stand_data"); reference them via @OnlyTags on definitions/instances
Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var, sound: Var, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Runtime wrapper entities should extend WrapperFakeEntity(EntityTypes., player)
Override applyProperty in wrapper to route per-entity properties first, then fall back to applyGenericEntityData and applyLivingEntityData
Do not manually manage viewers or metadata batching in wrappers; rely on WrapperFakeEntity (spawn, viewers, passengers, setNotifyAboutChanges)
For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Create a matching entry class XData(...) : GenericEntityData implementing type() and build(player)
Implement appliers: fun applyXData(entity: WrapperEntity, property: XProperty) using entity.metas { meta<...> { ... } } with a helpful error fallback
Hook per-...
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
🧠 Learnings (14)
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Use SimpleEntityDefinition + SimpleEntityInstance for straightforward cases; use Shared/Group/Individual AdvancedEntityInstance entries for complex lifecycles/show ranges
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Instance class <EntityName>Instance must be annotated with Entry and declare fields: id, name, definition: Ref<<EntityName>Definition>, spawnLocation, data (+OnlyTags as above), activity: Ref<out SharedEntityActivityEntry>
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Avoid mutating Bukkit entities directly; operate via WrapperEntity and Typewriter EntityProperty APIs
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Activities: implement GenericEntityActivityEntry and return an EntityActivity via create(context, currentLocation)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Keep visual and structural parity with similar entities (icons, colors, descriptions, class structure) like AllayEntity.kt or ArmorStandEntity.kt
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Access Var values via .get(player) where applicable in activities and entries
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Create a new vanilla entity file at entries/entity/minecraft/<EntityName>Entity.kt that defines <EntityName>Definition, <EntityName>Instance, and a private <EntityName>Entity wrapper
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var<String>, sound: Var<Sound>, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
🧬 Code graph analysis (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
engine/engine-core/src/main/kotlin/com/typewritermc/core/entries/EntryReference.kt (1)
emptyRef(6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt (1)
180-189: Ignore incorrect mismatch—EntityData is a PropertySupplier
EntityDataimplements PropertySupplier
, and Pair/List are covariant, so List<Pair<EntityData<>,Int>> satisfies List<Pair<PropertySupplier<>,Int>>. No conversion needed.
Likely an incorrect or invalid review comment.
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
Add support for spawning multiple instances of entities definitions. This allows users to spawn N copies of an entity either shared across all players or individually per player.
steveb05
force-pushed
the
feat/multiple-advanced-entity
branch
from
2a92c54 to
98ddf62
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt (4)
274-275: Replace ConcurrentHashMap with simpler collections.Based on a previous review, ConcurrentHashMaps are unnecessary here: "For the entity state we don't care about having an invalid value, because the next tick it will be fixed. And for the activity manager is created once and removed only in the dispose method."
Apply this diff:
- private val activityManagers = ConcurrentHashMap<UUID, List<ActivityManager<in IndividualActivityContext>>>() - private val lastStates = ConcurrentHashMap<UUID, MutableList<EntityState>>() + private var activityManagers = mutableMapOf<UUID, List<ActivityManager<in IndividualActivityContext>>>() + private val lastStates = mutableMapOf<UUID, MutableList<EntityState>>()Based on learnings
519-527: Add spawnPosition fallback to filter to avoid blocking initialization.When no managers exist yet for a group,
filterreturnsfalse, which may block initialization. Fall back to checking distance fromspawnPositionuntil managers are created.Apply this diff:
override fun filter(player: Player): Boolean { val groupId = group.groupId(player) ?: GroupId(player.uniqueId) - val managerList = activityManagers[groupId] ?: return false + val managerList = activityManagers[groupId] val showRangeSq = showRangeSq(player) + if (managerList == null || managerList.isEmpty()) { + return spawnPosition.distanceSqrt(player.location)?.let { it <= showRangeSq } ?: false + } + return managerList.any { manager -> manager.position.distanceSqrt(player.location)?.let { it <= showRangeSq } ?: false } }Based on learnings
549-566: Use manager-specific viewers instead of all group viewers in addedViewer.Currently
addedVieweris called withgroupViewers(groupId)which includes all players in the group, even those who don't yet have entities for this specific manager. This is too broad and should use only the viewers who can actually see this manager.Apply this diff to call
addedViewerafter updating the entities map with per-manager viewers:val newEntities = managerList.mapNotNull { manager -> manager.position.distanceSqrt(player.location)?.takeIf { it <= showRangeSq }?.let { - createDisplayEntity(player, manager).also { - manager.addedViewer(SharedActivityContext(instanceEntryRef, groupViewers(groupId)), player) - } + createDisplayEntity(player, manager) } } if (newEntities.isNotEmpty()) { entities[player.uniqueId] = newEntities + newEntities.forEach { entity -> + val viewers = entities.entries.mapNotNull { (pid, list) -> + val p = server.getPlayer(pid) + if (p != null && group.groupId(p) == groupId && list.any { it.activityManager === entity.activityManager }) p else null + } + entity.activityManager.addedViewer(SharedActivityContext(instanceEntryRef, viewers), player) + } }Based on learnings
311-327: Use IndividualActivityContext instead of SharedActivityContext.The Individual display variant should use
IndividualActivityContextfor viewer notifications, notSharedActivityContext. This ensures per-viewer activity semantics are maintained.Apply these changes:
In onPlayerFilterAdded (line 319):
manager.position.distanceSqrt(player.location)?.takeIf { it <= showRangeSq }?.let { createDisplayEntity(player, manager).also { - manager.addedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) + manager.addedViewer(IndividualActivityContext(instanceEntryRef, player), player) } }In tick() dynamic spawn (line 413) and despawn (line 422):
distance == null || distance > showRangeSq -> { currentEntity?.let { - manager.removedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) + manager.removedViewer(IndividualActivityContext(instanceEntryRef, player), player) it.dispose() } null } currentEntity != null -> currentEntity else -> { createDisplayEntity(player, manager).also { - manager.addedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) + manager.addedViewer(IndividualActivityContext(instanceEntryRef, player), player) } }In onPlayerFilterRemoved (line 443) and onPlayerRemove (line 453):
entities[player.uniqueId]?.forEach { entity -> - entity.activityManager.removedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) + entity.activityManager.removedViewer(IndividualActivityContext(instanceEntryRef, player), player) entity.dispose() }Based on learnings
Also applies to: 406-426, 440-447, 449-462
🧹 Nitpick comments (1)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt (1)
12-12: Prefer specific import over wildcard.The cleanup of unused imports (
Default,entityShowRange) is good. However, replacing the specificOptionalimport with a wildcardjava.util.*import reduces clarity and can introduce naming conflicts.Apply this diff to use a specific import:
-import java.util.* +import java.util.Optional
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt(1 hunks)extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/MultipleAdvancedEntityInstanceEntry.kt
🧰 Additional context used
📓 Path-based instructions (5)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt
📄 CodeRabbit inference engine (engine/AGENTS.md)
engine/{engine-core,engine-loader,engine-paper}/src/**/*.kt: Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin source files
Document public APIs with KDoc, focusing on when to use them rather than method-by-method descriptions
Avoid inline comments; refactor unclear code instead of adding explanations
Files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt
📄 CodeRabbit inference engine (.github/instructions/entity-extension.instructions.md)
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt: Annotate all entry classes with @entry(id, description, Colors, icon); use @tags on definition entries; use @OnlyTags on data lists to constrain allowed data types
Use snake_case, descriptive IDs (e.g., armor_stand_definition, allay_instance) and practical descriptions
Use icons from established packs (lucide:, ph:, fa6-solid:, etc.) and colors from com.typewritermc.core.books.pages.Colors
Apply data tagging: use "generic_entity_data" for generic properties, "living_entity_data" for living-specific, and add per-entity tag (e.g., "armor_stand_data"); reference them via @OnlyTags on definitions/instances
Each ...Property implements EntityProperty and declares a companion object : SinglePropertyCollectorSupplier to define defaults and merge behavior
Definition entries must implement SimpleEntityDefinition with fields: id, name, displayName: Var, sound: Var, data: List<Ref<EntityData<*>>>; create(player) returns the wrapper entity
Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Runtime wrapper entities should extend WrapperFakeEntity(EntityTypes., player)
Override applyProperty in wrapper to route per-entity properties first, then fall back to applyGenericEntityData and applyLivingEntityData
Do not manually manage viewers or metadata batching in wrappers; rely on WrapperFakeEntity (spawn, viewers, passengers, setNotifyAboutChanges)
For each new per-entity property, define data class XProperty(...): EntityProperty with companion SinglePropertyCollectorSupplier
Create a matching entry class XData(...) : GenericEntityData implementing type() and build(player)
Implement appliers: fun applyXData(entity: WrapperEntity, property: XProperty) using entity.metas { meta<...> { ... } } with a helpful error fallback
Hook per-...
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
extensions/*Extension/**
📄 CodeRabbit inference engine (extensions/AGENTS.md)
When adding a new extension, create its project directory with a name ending in "Extension" (e.g., MyPluginExtension)
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
extensions/{*Extension/src/**/*.kt,*Extension/**/*.kts}
📄 CodeRabbit inference engine (extensions/AGENTS.md)
Use 4 spaces for indentation and wrap lines at 120 characters in Kotlin code and Gradle Kotlin scripts
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
extensions/*Extension/src/**/*.kt
📄 CodeRabbit inference engine (extensions/AGENTS.md)
extensions/*Extension/src/**/*.kt: Keep functions short and focused; prefer guard clauses over nested conditionals
Document public APIs with KDoc, focusing on when to use them rather than restating method behavior
Avoid inline comments; refactor unclear code instead of adding explanatory comments
Files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
🧠 Learnings (10)
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Choose correct activity scope: SharedEntityActivityEntry for shared behavior/position, IndividualEntityActivityEntry for per-viewer behavior/position
Applied to files:
engine/engine-paper/src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.ktextensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Instance entries must implement SimpleEntityInstance (or an Advanced variant) with fields: id, name, definition: Ref<...Definition>, spawnLocation, data, and activity Ref of the correct scope
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Use SimpleEntityDefinition + SimpleEntityInstance for straightforward cases; use Shared/Group/Individual AdvancedEntityInstance entries for complex lifecycles/show ranges
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Ensure imports/types compile: Ref, Var, ConstVar, OnlyTags, and correct colors/icons/description in Entry
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Instance class <EntityName>Instance must be annotated with Entry and declare fields: id, name, definition: Ref<<EntityName>Definition>, spawnLocation, data (+OnlyTags as above), activity: Ref<out SharedEntityActivityEntry>
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Avoid mutating Bukkit entities directly; operate via WrapperEntity and Typewriter EntityProperty APIs
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Annotate all entry classes with Entry(id, description, Colors, icon); use Tags on definition entries; use OnlyTags on data lists to constrain allowed data types
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:11:05.217Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.instructions.md:0-0
Timestamp: 2025-08-24T11:11:05.217Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/**/*.kt : Activities: implement GenericEntityActivityEntry and return an EntityActivity via create(context, currentLocation)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Keep visual and structural parity with similar entities (icons, colors, descriptions, class structure) like AllayEntity.kt or ArmorStandEntity.kt
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
📚 Learning: 2025-08-24T11:10:07.264Z
Learnt from: CR
PR: gabber235/Typewriter#0
File: .github/instructions/entity-extension.add-entity.instructions.md:0-0
Timestamp: 2025-08-24T11:10:07.264Z
Learning: Applies to extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/entity/minecraft/*Entity.kt : Entry IDs for both definition and instance must be snake_case and unique (e.g., allay_definition, allay_instance)
Applied to files:
extensions/EntityExtension/src/main/kotlin/com/typewritermc/entity/entries/instance/AdvancedEntityInstanceEntry.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
.../src/main/kotlin/com/typewritermc/engine/paper/entry/entity/MultipleAudienceEntityDisplay.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
There are quite a few large functions that basically do the same thing.
It might be wise to extract this into a more generic function that all three can call, passing callbacks for the specific tasks.
Or at least break each instance into more functions, because it can be hard to figure out what each part is for and what it's doing.
Also, there's often a lot of indented code, a lot of nested callbacks, if statements, etc. At some point try to extract and invert.
| val newEntities = managerList.mapNotNull { manager -> | ||
| manager.position.distanceSqrt(player.location)?.takeIf { it <= showRangeSq }?.let { | ||
| createDisplayEntity(player, manager).also { | ||
| manager.addedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) |
There was a problem hiding this comment.
This should be an individual context!
| if (currentCount < currentEntities.size) { | ||
| currentEntities.drop(currentCount).forEach { entity -> | ||
| entity.activityManager.removedViewer( | ||
| SharedActivityContext(instanceEntryRef, listOf(player)), |
There was a problem hiding this comment.
This should be an individual context!
| when { | ||
| distance == null || distance > showRangeSq -> { | ||
| currentEntity?.let { | ||
| manager.removedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) |
There was a problem hiding this comment.
This should be an individual context!
| currentEntity != null -> currentEntity | ||
| else -> { | ||
| createDisplayEntity(player, manager).also { | ||
| manager.addedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) |
There was a problem hiding this comment.
This should be an individual context!
| override fun onPlayerFilterRemoved(player: Player) { | ||
| super.onPlayerFilterRemoved(player) | ||
| entities[player.uniqueId]?.forEach { entity -> | ||
| entity.activityManager.removedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) |
There was a problem hiding this comment.
This should be an individual context!
| override fun tick() { | ||
| consideredPlayers.forEach { it.refresh() } | ||
|
|
||
| consideredPlayers.forEach { player -> | ||
| val currentCount = count(player.uniqueId) | ||
| val currentManagers = activityManagers[player.uniqueId] ?: emptyList() | ||
|
|
||
| if (currentCount != currentManagers.size) { | ||
| val newManagers = when { | ||
| currentCount > currentManagers.size -> { | ||
| val spawnPos = spawnPosition.get(player) | ||
| currentManagers + List(currentCount - currentManagers.size) { | ||
| val context = IndividualActivityContext(instanceEntryRef, player) | ||
| val activity = activityCreator.create(context, spawnPos.toProperty()) | ||
| ActivityManager(activity).apply { initialize(context) } | ||
| } | ||
| } | ||
|
|
||
| else -> { | ||
| currentManagers.drop(currentCount).forEach { manager -> | ||
| manager.dispose(IndividualActivityContext(instanceEntryRef, player)) | ||
| } | ||
| currentManagers.take(currentCount) | ||
| } | ||
| } | ||
|
|
||
| activityManagers[player.uniqueId] = newManagers | ||
|
|
||
| val states = lastStates[player.uniqueId] ?: mutableListOf() | ||
| when { | ||
| currentCount > states.size -> { | ||
| states.addAll(List(currentCount - states.size) { EntityState() }) | ||
| } | ||
|
|
||
| currentCount < states.size -> { | ||
| while (states.size > currentCount) states.removeAt(states.lastIndex) | ||
| } | ||
| } | ||
| lastStates[player.uniqueId] = states | ||
|
|
||
| val currentEntities = entities[player.uniqueId] ?: emptyList() | ||
| if (currentCount < currentEntities.size) { | ||
| currentEntities.drop(currentCount).forEach { entity -> | ||
| entity.activityManager.removedViewer( | ||
| SharedActivityContext(instanceEntryRef, listOf(player)), | ||
| player | ||
| ) | ||
| entity.dispose() | ||
| } | ||
| entities[player.uniqueId] = currentEntities.take(currentCount) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| activityManagers.forEach { (playerId, managerList) -> | ||
| val player = server.getPlayer(playerId) ?: return@forEach | ||
| val entityList = entities[playerId] ?: emptyList() | ||
| val states = lastStates[playerId] ?: return@forEach | ||
|
|
||
| managerList.forEachIndexed { index, manager -> | ||
| val entity = entityList.find { it.activityManager === manager } | ||
| val isViewing = entity != null | ||
| val entityState = entity?.state?.also { | ||
| if (index < states.size) states[index] = it | ||
| } ?: states.getOrElse(index) { EntityState() } | ||
|
|
||
| manager.tick(IndividualActivityContext(instanceEntryRef, player, isViewing, entityState)) | ||
| } | ||
| } | ||
|
|
||
| val showRanges = consideredPlayers.associateWith { showRangeSq(it) } | ||
|
|
||
| consideredPlayers.forEach { player -> | ||
| val showRangeSq = showRanges[player] ?: return@forEach | ||
| val managerList = managers(player.uniqueId) | ||
| val currentEntities = entities[player.uniqueId] ?: emptyList() | ||
|
|
||
| val newEntities = managerList.mapNotNull { manager -> | ||
| val distance = manager.position.distanceSqrt(player.location) | ||
| val currentEntity = currentEntities.find { it.activityManager === manager } | ||
|
|
||
| when { | ||
| distance == null || distance > showRangeSq -> { | ||
| currentEntity?.let { | ||
| manager.removedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) | ||
| it.dispose() | ||
| } | ||
| null | ||
| } | ||
|
|
||
| currentEntity != null -> currentEntity | ||
| else -> { | ||
| createDisplayEntity(player, manager).also { | ||
| manager.addedViewer(SharedActivityContext(instanceEntryRef, listOf(player)), player) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (newEntities.isNotEmpty()) { | ||
| entities[player.uniqueId] = newEntities | ||
| } else { | ||
| entities.remove(player.uniqueId) | ||
| } | ||
| } | ||
|
|
||
| entities.values.forEach { entityList -> | ||
| entityList.forEach { it.tick() } | ||
| } | ||
| } |
There was a problem hiding this comment.
This should be split up into more functions, at least the three clearly separate blocks. But probably a few more for removing a viewer from an entity and adding one, as you do that repeatedly, I think.
Because now its difficult for me to figure out what different parts are doing here.
| override fun tick() { | ||
| consideredPlayers.forEach { it.refresh() } | ||
|
|
||
| activityManagers.forEachIndexed { index, manager -> | ||
| val viewers = viewersForManager(manager) | ||
| val state = viewers.firstNotNullOfOrNull { viewer -> | ||
| entities[viewer.uniqueId] | ||
| ?.find { it.activityManager === manager } | ||
| ?.state | ||
| ?.also { lastStates[index] = it } | ||
| } ?: lastStates.getOrElse(index) { EntityState() } | ||
|
|
||
| manager.tick(SharedActivityContext(instanceEntryRef, viewers, state)) | ||
| } | ||
|
|
||
| val showRanges = consideredPlayers.associateWith { showRangeSq(it) } | ||
|
|
||
| consideredPlayers.forEach { player -> | ||
| val rangeSq = showRanges[player] ?: return@forEach | ||
| val currentEntities = entities[player.uniqueId] ?: emptyList() | ||
|
|
||
| val managersToAdd = mutableListOf<ActivityManager<*>>() | ||
| val managersToRemove = mutableListOf<ActivityManager<*>>() | ||
| val entitiesToRemove = mutableListOf<DisplayEntity>() | ||
|
|
||
| val newEntities = activityManagers.mapNotNull { manager -> | ||
| val distance = manager.position.distanceSqrt(player.location) | ||
| val currentEntity = currentEntities.find { it.activityManager === manager } | ||
|
|
||
| when { | ||
| distance == null || distance > rangeSq -> { | ||
| currentEntity?.let { | ||
| entitiesToRemove.add(it) | ||
| managersToRemove.add(manager) | ||
| } | ||
| null | ||
| } | ||
|
|
||
| currentEntity != null -> currentEntity | ||
| else -> { | ||
| managersToAdd.add(manager) | ||
| createDisplayEntity(player, manager) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (newEntities.isNotEmpty()) { | ||
| entities[player.uniqueId] = newEntities | ||
| } else { | ||
| entities.remove(player.uniqueId) | ||
| } | ||
|
|
||
| managersToRemove.forEach { manager -> | ||
| val viewers = viewersForManager(manager) | ||
| manager.removedViewer(SharedActivityContext(instanceEntryRef, viewers), player) | ||
| } | ||
|
|
||
| entitiesToRemove.forEach { it.dispose() } | ||
| managersToAdd.forEach { manager -> | ||
| val viewers = viewersForManager(manager) | ||
| manager.addedViewer(SharedActivityContext(instanceEntryRef, viewers), player) | ||
| } | ||
| } | ||
|
|
||
| entities.values.flatten().forEach { it.tick() } | ||
| } |
There was a problem hiding this comment.
This should be split up into more functions, at least the three clearly separate blocks. But probably a few more for removing a viewer from an entity and adding one, as you do that repeatedly, I think.
Because now its difficult for me to figure out what is going on here.
| override fun tick() { | ||
| consideredPlayers.forEach { it.refresh() } | ||
|
|
||
| activityManagers.forEach { (groupId, managerList) -> | ||
| val viewers = groupViewers(groupId) | ||
| val states = lastStates[groupId] ?: return@forEach | ||
|
|
||
| managerList.forEachIndexed { index, manager -> | ||
| val entityState = viewers.firstNotNullOfOrNull { viewer -> | ||
| entities[viewer.uniqueId]?.find { it.activityManager === manager }?.state | ||
| }?.also { | ||
| if (index < states.size) states[index] = it | ||
| } ?: states.getOrElse(index) { EntityState() } | ||
|
|
||
| manager.tick(SharedActivityContext(instanceEntryRef, viewers, entityState)) | ||
| } | ||
| } | ||
|
|
||
| val showRanges = consideredPlayers.associateWith { showRangeSq(it) } | ||
|
|
||
| consideredPlayers.forEach { player -> | ||
| val groupId = group.groupId(player) ?: GroupId(player.uniqueId) | ||
| val showRangeSq = showRanges[player] ?: return@forEach | ||
| val managerList = activityManagers[groupId] ?: return@forEach | ||
| val currentEntities = entities[player.uniqueId] ?: emptyList() | ||
|
|
||
| val newEntities = managerList.mapNotNull { manager -> | ||
| val distance = manager.position.distanceSqrt(player.location) | ||
| val currentEntity = currentEntities.find { it.activityManager === manager } | ||
|
|
||
| when { | ||
| distance == null || distance > showRangeSq -> { | ||
| currentEntity?.let { | ||
| manager.removedViewer( | ||
| SharedActivityContext(instanceEntryRef, groupViewers(groupId)), | ||
| player | ||
| ) | ||
| it.dispose() | ||
| } | ||
| null | ||
| } | ||
|
|
||
| currentEntity != null -> { | ||
| currentEntity | ||
| } | ||
|
|
||
| else -> { | ||
| createDisplayEntity(player, manager).also { | ||
| manager.addedViewer(SharedActivityContext(instanceEntryRef, groupViewers(groupId)), player) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (newEntities.isNotEmpty()) { | ||
| entities[player.uniqueId] = newEntities | ||
| } else { | ||
| entities.remove(player.uniqueId) | ||
| } | ||
| } | ||
|
|
||
| entities.values.forEach { entityList -> | ||
| entityList.forEach { it.tick() } | ||
| } | ||
| } |
There was a problem hiding this comment.
This should be split up into more functions, at least the three clearly separate blocks. But probably a few more for removing a viewer from an entity and adding one, as you do that repeatedly, I think.
Because now its difficult for me to figure out what is going on here.
| override fun tick() { | ||
| consideredPlayers.forEach { it.refresh() } | ||
|
|
||
| activityManagers.forEach { (groupId, managerList) -> | ||
| val viewers = groupViewers(groupId) | ||
| val states = lastStates[groupId] ?: return@forEach | ||
|
|
||
| managerList.forEachIndexed { index, manager -> | ||
| val entityState = viewers.firstNotNullOfOrNull { viewer -> | ||
| entities[viewer.uniqueId]?.find { it.activityManager === manager }?.state | ||
| }?.also { | ||
| if (index < states.size) states[index] = it | ||
| } ?: states.getOrElse(index) { EntityState() } | ||
|
|
||
| manager.tick(SharedActivityContext(instanceEntryRef, viewers, entityState)) | ||
| } | ||
| } | ||
|
|
||
| val showRanges = consideredPlayers.associateWith { showRangeSq(it) } | ||
|
|
||
| consideredPlayers.forEach { player -> | ||
| val groupId = group.groupId(player) ?: GroupId(player.uniqueId) | ||
| val showRangeSq = showRanges[player] ?: return@forEach | ||
| val managerList = activityManagers[groupId] ?: return@forEach | ||
| val currentEntities = entities[player.uniqueId] ?: emptyList() | ||
|
|
||
| val newEntities = managerList.mapNotNull { manager -> | ||
| val distance = manager.position.distanceSqrt(player.location) | ||
| val currentEntity = currentEntities.find { it.activityManager === manager } | ||
|
|
||
| when { | ||
| distance == null || distance > showRangeSq -> { | ||
| currentEntity?.let { | ||
| manager.removedViewer( | ||
| SharedActivityContext(instanceEntryRef, groupViewers(groupId)), | ||
| player | ||
| ) | ||
| it.dispose() | ||
| } | ||
| null | ||
| } | ||
|
|
||
| currentEntity != null -> { | ||
| currentEntity | ||
| } | ||
|
|
||
| else -> { | ||
| createDisplayEntity(player, manager).also { | ||
| manager.addedViewer(SharedActivityContext(instanceEntryRef, groupViewers(groupId)), player) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (newEntities.isNotEmpty()) { | ||
| entities[player.uniqueId] = newEntities | ||
| } else { | ||
| entities.remove(player.uniqueId) | ||
| } | ||
| } | ||
|
|
||
| entities.values.forEach { entityList -> | ||
| entityList.forEach { it.tick() } | ||
| } | ||
| } |
There was a problem hiding this comment.
Also is this doing everything it needs to? Because it feels much shorter than the other 2 tick methods...
|
|
||
| entities.remove(player.uniqueId)?.forEach { it.dispose() } | ||
|
|
||
| // If no players are considered for this group, we can remove the activity managers |
There was a problem hiding this comment.
This is a redundant comment.
A better comment would be:
When there is nobody that can view an entity, we no longer need to track its activity.
It might seem like a subtle difference, but your comment describes the code while mine explains why this is here.
There was a problem hiding this comment.
This was copied from the group audience entity display
It might be worth to change it also there.
2 participants
This PR enables spawning multiple instances of the same entity, opening up several use cases that weren't possible before.
Previously, users had to define a separate entity instance for every entity they wanted to spawn, which got unwieldy fast when dealing with dozens of similar entities. Now they can just specify a multiplier.
The proximity detection ensures that when a player interacts with or queries one of these entities, they get the closest one. Distance-based rendering keeps performance in check by only spawning entities when players are close enough to see them.
Also includes minor cleanup to remove unused imports from AdvancedEntityInstanceEntry file.
Summary by CodeRabbit
New Features
Style