-
Notifications
You must be signed in to change notification settings - Fork 11
PylonTickingEntity #483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
PylonTickingEntity #483
Conversation
pylon-core/src/main/kotlin/io/github/pylonmc/pylon/core/entity/base/PylonTickableEntity.kt
Outdated
Show resolved
Hide resolved
pylon-core/src/main/kotlin/io/github/pylonmc/pylon/core/entity/base/PylonTickableEntity.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Seggan <seggan21@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a few things missing here - I would suggest including the following in this PR, but if you don't want to do them in this PR just make sure open issues for them so we can keep track:
- Needs async ticking option as we have for blocks
- Needs error handling (we have error handling set up for other entity interfaces)
- Currently you're using a getter for
tickDelay, but the convention with these sorts of interface variables is to usesetTickIntervalandsetAsyncand so on. The reason for this is to avoid having loads of getters for trivial properties which became quite a problem especially w/ the fluid stuff. It should hopefully be fairly straightforward as you can replicate what ticking blocks do.
(also docs :P - you can just do something similar to the block tickers for this)
pylon-core/src/main/kotlin/io/github/pylonmc/pylon/core/entity/base/PylonTickableEntity.kt
Outdated
Show resolved
Hide resolved
LordIdra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small thing but otherwise I'm happy
pylon-core/src/main/kotlin/io/github/pylonmc/pylon/core/entity/EntityStorage.kt
Outdated
Show resolved
Hide resolved
Seggan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than Idra comments LGTM
...re/src/main/kotlin/io/github/pylonmc/pylon/core/datatypes/TickingEntityPersistentDataType.kt
Outdated
Show resolved
Hide resolved
...re/src/main/kotlin/io/github/pylonmc/pylon/core/datatypes/TickingEntityPersistentDataType.kt
Outdated
Show resolved
Hide resolved
pylon-core/src/main/kotlin/io/github/pylonmc/pylon/core/entity/EntityListener.kt
Show resolved
Hide resolved
pylon-core/src/main/kotlin/io/github/pylonmc/pylon/core/entity/EntityStorage.kt
Outdated
Show resolved
Hide resolved
pylon-core/src/main/kotlin/io/github/pylonmc/pylon/core/entity/EntityStorage.kt
Outdated
Show resolved
Hide resolved
pylon-core/src/main/kotlin/io/github/pylonmc/pylon/core/event/PylonEntityAddEvent.kt
Show resolved
Hide resolved
pylon-core/src/main/kotlin/io/github/pylonmc/pylon/core/entity/base/PylonTickingEntity.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry 1 big issue that I missed in my previous pass
| @JvmSynthetic | ||
| internal fun logEventHandleErrTicking(e: Exception, entity: PylonEntity<*>) { | ||
| PylonCore.logger.severe("Error when handling ticking entity(${entity.key}, ${entity.uuid}, ${entity.entity.location}): ${e.localizedMessage}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 small things for clarity. no event handling going on here so would be best to call it something like logTickingErr and change the message to "Error when ticking entity"
| @EventHandler | ||
| private fun onSerialize(event: PylonEntityAddEvent) { //todo serialize | ||
| val entity = event.pylonEntity | ||
| if (entity is PylonTickingEntity) { | ||
| entity.entity.persistentDataContainer.set(tickingEntityKey, PylonSerializers.TICKING_ENTITY_DATA, tickingEntities[entity]!!) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't serialization be happening on serialize, not on add? Just checked and there's no event for this... I would add PylonEntitySerialize and PylonEntityDeserialize events here for consistency with blocks, because AFAICT this will not work properly if only called on PylonEntityAdd (won't respect tick inteval set later)
Fixes #456