Skip to content
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

Bevy 0.15 and latest ggrs main #114

Merged
merged 4 commits into from
Dec 19, 2024
Merged

Bevy 0.15 and latest ggrs main #114

merged 4 commits into from
Dec 19, 2024

Conversation

johanhelsing
Copy link
Collaborator

@johanhelsing johanhelsing commented Dec 15, 2024

This is #113, but with the extra changes needed after gschup/ggrs#82 by @caspark got merged

  • [ ] Change to ggrs crates.io release once it's out
  • Fix panic in particles example

haihala and others added 4 commits December 4, 2024 19:24
This is what bevy 0.15 migration docs suggest
Default, Serialize and Deserialize instead of bytemuck traits.
@johanhelsing
Copy link
Collaborator Author

johanhelsing commented Dec 15, 2024

Hmmm...

This leads to a panic for me:

cargo run --release --example particles -- --local-port 7000 --players localhost 127.0.0.1:7001 --input-delay 0
cargo run --release --example particles -- --local-port 7001 --players localhost 127.0.0.1:7000 --input-delay 0

As soon as one of the clients gets input and tries to roll back

b…/examples bevy-0.15 …1 cargo run --release --example particles -- --local-port 7000 --players localhost 127.0.0.1:7001 --input-delay 0
    Finished `release` profile [optimized] target(s) in 0.41s
     Running `J:\dev\bevy_ggrs\target\release\examples\particles.exe --local-port 7000 --players localhost 127.0.0.1:7001 --input-delay 0`
2024-12-15T10:27:15.813742Z  INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Windows 11 Pro", kernel: "22631", cpu: "AMD Ryzen 9 3900X 12-Core Processor", core_count: "12", memory: "31.9 GiB" }
2024-12-15T10:27:16.060679Z  INFO bevy_render::renderer: AdapterInfo { name: "NVIDIA GeForce RTX 2070 SUPER", vendor: 4318, device: 7812, device_type: DiscreteGpu, driver: "NVIDIA", driver_info: "560.94", backend: Vulkan }
2024-12-15T10:27:16.209275Z  INFO bevy_winit::system: Creating new window "GGRS particles stress test" (0v1#4294967296)
2024-12-15T10:27:16.453136Z  INFO particles: GGRS event: Synchronizing { addr: 127.0.0.1:7001, total: 5, count: 1 }
2024-12-15T10:27:16.455488Z  INFO particles: GGRS event: Synchronizing { addr: 127.0.0.1:7001, total: 5, count: 2 }
2024-12-15T10:27:16.487348Z  INFO particles: GGRS event: Synchronizing { addr: 127.0.0.1:7001, total: 5, count: 3 }
2024-12-15T10:27:16.520277Z  INFO particles: GGRS event: Synchronizing { addr: 127.0.0.1:7001, total: 5, count: 4 }
2024-12-15T10:27:16.554322Z  INFO particles: GGRS event: Synchronized { addr: 127.0.0.1:7001 }
Prediction Threshold reached. Skipping on frame 79
Prediction Threshold reached. Skipping on frame 88
Prediction Threshold reached. Skipping on frame 88
thread 'main' panicked at C:\Users\Johan\.cargo\git\checkouts\ggrs-151f8bd9eb75dc5d\534170e\src\sessions\p2p_session.rs:952:44:
cell not found!: frame 80
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_ggrs::schedule_systems::run_ggrs_schedules<bevy_ggrs::GgrsConfig<u8>>`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!
error: process didn't exit successfully: `J:\dev\bevy_ggrs\target\release\examples\particles.exe --local-port 7000 --players localhost 127.0.0.1:7001 --input-delay 0` (exit code: 101)

Might be related to latest changes in ggrs?

Ideas @caspark ?

@caspark
Copy link

caspark commented Dec 15, 2024

@johanhelsing this looks like the crash when rolling back while at the max prediction that gschup/ggrs#79 introduced, which then lived in master for a couple weeks until I fixed it in gschup/ggrs#88 (see 1st bullet point). Based on timing of your comment and the merging of that 2nd PR I am guessing you didn't have that fix PR's commits included when testing?

If you do still get the issue with the fix PR's commits then that probably means I stuffed something up (or failed to completely fix #79 correctly), in which case I'd love to know more about it - though ideally in a non-bevy_ggrs example as I don't use bevy so haven't looked at the bevy_ggrs code at all yet :)

@johanhelsing
Copy link
Collaborator Author

Pretty sure i did cargo update today. Can't check now though

@gschup
Copy link
Owner

gschup commented Dec 15, 2024

... that gschup/ggrs#79 introduced...
...or failed to completely fix #79 ...

You make it sound like I'm the evil guy here ;)

@caspark
Copy link

caspark commented Dec 15, 2024

Haha, yes @gschup , you're the bad guy for porting GGPO to Rust while figuring out a much saner design in the process, and then maintaining & improving upon it for the last 3 years, entirely for free! Only a true monster would do that.

Kidding aside: just to be clear, I didn't mean to paint you as "the bad guy", so if I came across that way, sorry: I apologize for my choice of wording. I was just trying to say that I thought that particular crash wasn't due to my changes, and provide sufficient context as to why (though it is entirely possible that I have broken ggrs in other ways).

@johanhelsing
Copy link
Collaborator Author

johanhelsing commented Dec 17, 2024

I did a bisect, and this was caused by gschup/ggrs@b13e7fc...

And then fixed again in gschup/ggrs@dc25e8d

So your guesses were spot on @caspark 👍

So this means, don't hold up on the ggrs release because of this!

@johanhelsing
Copy link
Collaborator Author

I just realized that we already depend on ggrs main in bevy_ggrs main, so there is no reason not to merge this I think?

We can just do another minor PR when doing the crates relases?

@johanhelsing johanhelsing marked this pull request as ready for review December 17, 2024 09:49
@johanhelsing johanhelsing requested a review from gschup December 17, 2024 09:49
@haihala
Copy link
Contributor

haihala commented Dec 18, 2024

Not sure if it's something to do with this thing, but I'm having this issue where I get ghosts of gltf meshes that don't appear in the world when I go over it with bevy-egui-inspector.

According to someone on the bevy discord, this could be a desync between the main and the render worlds. It happens when call despawn_recursive on an entity that has a child that has a SceneRoot that points to a gltf with a few more levels of hierarchy.

@caspark
Copy link

caspark commented Dec 18, 2024

ghosts of gltf meshes that don't appear in the world

I can't speak for the changes in bevy_ggrs as I'm not familiar with it, but that at least seems unlikely to be caused by the unreleased changes in ggrs itself. Try a synctest session with checkdistance of 0 - if you still get the issue, it's almost certainly a bevy bug rather than a ggrs or bevy_ggrs bug.

@haihala
Copy link
Contributor

haihala commented Dec 19, 2024

ghosts of gltf meshes that don't appear in the world

I can't speak for the changes in bevy_ggrs as I'm not familiar with it, but that at least seems unlikely to be caused by the unreleased changes in ggrs itself. Try a synctest session with checkdistance of 0 - if you still get the issue, it's almost certainly a bevy bug rather than a ggrs or bevy_ggrs bug.

Did that, ghosts didn't show up (yay?). I did just now notice that I'm getting some logs in the console, but only when check distance is not zero:

2024-12-19T06:06:17.373116Z  WARN ggrs{name="HandleRequests"}:schedule{name="AdvanceWorld"}: bevy_ecs::world: error[B0003]: /home/hajhawa/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_hierarchy-0.15.0/src/hierarchy.rs:49:19: Could not despawn entity 798v1#4294968094 because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003
2024-12-19T06:06:17.375426Z  WARN bevy_hierarchy::valid_parent_check_plugin: warning[B0004]: The Pickup model entity with the InheritedVisibility component has a parent without InheritedVisibility.
This will cause inconsistent behaviors! See: https://bevyengine.org/learn/errors/b0004
2024-12-19T06:06:17.375446Z  WARN bevy_hierarchy::valid_parent_check_plugin: warning[B0004]: The Pickup model entity with the GlobalTransform component has a parent without GlobalTransform.
This will cause inconsistent behaviors! See: https://bevyengine.org/learn/errors/b0004
2024-12-19T06:06:17.389048Z  WARN ggrs{name="HandleRequests"}:schedule{name="AdvanceWorld"}: bevy_ecs::world: error[B0003]: /home/hajhawa/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_hierarchy-0.15.0/src/hierarchy.rs:49:19: Could not despawn entity 798v1#4294968094 because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003

Probably related to that. Is there like a best practice for nested entities with ggrs that I should be aware of? I mean stuff like, should I call .add_rollback on the child? What about the children the SceneRoot brings in? Should I avoid despawn_recursive on them?

@johanhelsing
Copy link
Collaborator Author

Yes, the children also need add_rollback, we don't do despawn recursive when rolling back entities, so all entities need in the hierarchy to have the rollback component added

commands.entity(current_entity).despawn();

Are you not getting these issues on bevy 0.14

@haihala
Copy link
Contributor

haihala commented Dec 19, 2024

I feel like this may be on my end, moved the conversation over to discord. Would appreciate any help in my descend to madness over yonder.

@gschup gschup merged commit 33cf3ae into gschup:main Dec 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants