&mut Self in Entity & dynamic Downcasting#227
Draft
unschlagbar wants to merge 151 commits into
Draft
Conversation
Integrates upstream's "Entity refactor part 2" (Steel-Foundation#225) and the other 12 master commits (26.2 protocol, AABB/vector refactor, worldgen perf, Docker, custom auth, sigterm/log handling) into this branch's locked `&mut self` / per-entity-lock + ServerPlayer model. Resolution strategy (940 conflict hunks across 95 files): - Lower crates (macros/protocol/registry/utils): took master's APIs; kept the branch's `entity_behavior` codegen island (build_block_items, build/common.rs, classes.json) since the branch entity impls depend on it. - Entity/player architecture: kept the branch's `&mut self` locked model (per the project's deadlock-fix work); ported master's new content to it. - Adapted seams: AABB method API (move_vec/by -> translate, field -> method accessors), restructured protocol packets (CAddEntity/SMoveVehicle/ CSetEntityMotion/CPlayerPosition/CSound/CLogin), BlockStateId shape API (get_*_shape -> get_static_*_shape), SoundEventHolder, Option<GameType> previous-gamemode, new enchantment effect variants, IVec3/JigsawJunction serialization, and entity construction (Arc::new(X::new) -> X::new + lock_entity/downcast). - Absorbed master-new content adapted to the locked model: potent_sulfur block + geyser block-entity, sulfur_spike, chat spam-throttler. - Deferred upstream features that require master's incompatible entity-manager / worlds refactor: chunk entity-visibility (no-op shim) and the setworldspawn command (commented out). cargo build --workspace: clean. cargo test -p steel-core: 980 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Brings in 7 upstream commits (cocoa, sponge, rooted dirt, hanging moss, simple vegetation, non-vanilla view-distance + custom chunk-gen threads, docker/SWH). Single conflict in server/mod.rs: kept the branch's locked apply_first_visit_defaults spawn flow and added upstream's chunk-gen thread-count helpers; dropped upstream's PreparedSpawn/apply_default_spawn restructuring (incompatible with the branch spawn model). Fixed view-distance seam in connection.rs (self.config -> self.config()). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This pull request has conflicts with the base branch "master". Please resolve those so we can test out your changes. |
The fix was applied to the working tree but missed the previous merge commit (it was unstaged), leaving the pushed branch non-compiling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
entity refactor have been merge already |
|
Conflicts have been resolved! 🎉 |
b45cc46 to
879c0d9
Compare
|
This pull request has conflicts with the base branch "master". Please resolve those so we can test out your changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The goal of the pr
The current entities system tries to avoid deadlocks and a well thought computation order by making all entities completely interior mutable. part of the reason is probably also that most of the devs come from pumpkin where it was practiced similar. The idea is also that all threads can access all entities without having to worry that data may be inaccessible.
Chunk gen in steel is already very fast and very strong parallelized, the speed however comes from the fact that multiple threads can work on there own chunk, there is basically no dependency between the chunks for the heavy part. If multiple threads would work on the same chunk, the sync cost would be almost larger than the multi threading advantage. The same thing applies to entities as well. Entities are a complete unit just like a chunk. for every tick one entities is supposed to only tick from one thread. the interactions between the entities are up to 3 extra locks for complex entities. compared to way over 10 locks and an ugly amount of atomics loads and cell locks.
In my opinion this new entity handling way, is the most important thing that would easily lift steel up to the best mc rust server with a future where writing new and custom entities is just as fun as writing other rust code.
Implementation
SharedEntityis nowArc<EntityBase>. Having an arc here means that this is the root of the entity.EntityBasenow has two more members:For normal Entities the entity member gets the inner entity assign at the initialization. It doesnt need to be an arc since its not the root and the dyn Entity is not supposed to be stored any where. So the entities gets stored as SharedEntities but in fns params its often &mut dyn Entity, just not in the fns where you need a downcast (more on that later).
Players are not owned by the
SharedEntity. Since player have parts that can and must be accessed by multiple thread, i moved all these part into a new StructServerPlayer(name comes from vanilla).ServerPlayeris the root and owns the player, so thats whySharedEntityonly has a weak to it. its not final what is supposed to be inServerPlayerand what in player (AI decided it for now).Playeralso has anWeaktoServerPlayer. The goal on that part is to move all network handling toServerPlayeror maybe even to connection (to replicated vanillasServerGamePacketListenerImpl) that would also allow for better custom protocols like suggested here: Issue 79.You can think of
Playeras the entity part of the player that ticks movement and that stuff,ServerPlayeris the other part that has more power and is interior mutable.There is a new dowcasting system that removes the need for entity trait methods like: as_experience_orb, as_item_entity.
Its available to use through
let entity: Option<T> = entity_base.lock_entity().downcast::<T>(). To verify that self actually is T we take theEntityTypeof self and compare the identifier to the identifier derive by this macrofor entities where the identifier is different than the class
for entities where its the same
that also means that it is going to be forbidden, that future mods implement the same entity with the same namespace, just like vanilla.
How to take care of correct order
With this pr, we must make sure for entity x that in its tick function there is no re locking like this
self.base().lock_entity()in the entire call chain. That means no all functions in the chain should take &mut dyn Entity if entity specific methods are called otherwiseEntityBaseis fine.Todo
Now that i have the diff i should be able to clean every mess up that ai leaved (over 300 entity tests that ai fixed).
The first and best thing is to iterate on this until im satisfied, no idea what exactly needs to be done. Going to update the description once i have it more specifically.
Remove every single
MutexfromLivingEntityBase, reorder/group members to align with vanilla and change the signature of every affected method to take mut.Bring this pr up to date with Master