[Merged by Bors] - feat: reimplement stomp boots#595
[Merged by Bors] - feat: reimplement stomp boots#595legendiguess wants to merge 1 commit intofishfolk:mainfrom legendiguess:reimplement-stomp-boots
Conversation
zicklag
left a comment
zicklag
left a comment
There was a problem hiding this comment.
Left a comment about the entity comparison and how that plays into the bigger picture a little.
Let me know if you need more clarification over chat. It's something I'd like to improve the ergonomics/confusingness of later I think.
core/src/player.rs
Outdated
| ); | ||
| } | ||
|
|
||
| attachments_to_remove.iter().for_each(|ent| { |
There was a problem hiding this comment.
I think it'd be better to remove attachments when the player despawns in the player event handler here:
core/src/player.rs
Outdated
| inventories.insert(entity, default()); | ||
|
|
||
| for (attachment_entity, attachment) in entities.iter_with(&mut attachments) { | ||
| if attachment.entity.index() == entity.index() { |
There was a problem hiding this comment.
Entities should be able to be compared directly without comparing their indexes. That is also important to make sure that they don't have the same index but a different generation.
After testing I can see where the issue with just comparing th entities is.
So, when comparing the entities directly, the reason that this doesn't end up deleting the attachment is because when the player is killed, the old entities dies, and he spawns with a new entity. So even though the entity happens to have the same index, it is actually a new entity.
Then we run into the question of why the attachment continues to follow the newly spawned player when it has a different entity. This has to do with a nuance in the way bones_ecs works.
When you do atlas_sprites.get(entity), unfortunately, it only checks the entity index, and if it finds component data at that index, it returns it. So if we have an entity such as attachment.entity, and that entity has been killed, the only way for us to know is if we manually check !entities.is_alive(entity) before we call atlas_sprites.get(entity). So technically, we should, in the attachment system, check that the entity is still alive, before attaching to it.
That would mean that the attachment would behave properly and just stop moving, if the entity it was attached to is killed.
Then, if we want to fix the attachment despawning issue, we just need to despawn the attachments for a player when he is despawned in the handle_player_events system, not when he is respawned. That way, the player entity is still alive, and we do proper entity comparison to find it's attachments, right before killing the player entity.
There was a problem hiding this comment.
Here's an issue I just opened that gives an overview of the confusing aspect of entity liveness: fishfolk/bones#82.
|
Awesome, thanks! bors merge |
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
2 participants
No description provided.