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

Panic when syncing many entities #12

Closed
mvesterli opened this issue Sep 29, 2018 · 3 comments · Fixed by #17
Closed

Panic when syncing many entities #12

mvesterli opened this issue Sep 29, 2018 · 3 comments · Fixed by #17
Assignees
Labels
bug Something isn't working
Milestone

Comments

@mvesterli
Copy link
Contributor

If the synchronization message is greater than supported by the socket, it fails to send and therefore panics.
On windows 10 this limit is 64kB, which is ~2500 empty entities and significantly less when components are involved.

This code causes the error on my machine.

extern crate amethyst;
extern crate amethyst_editor_sync;

use amethyst::prelude::*;
use amethyst_editor_sync::*;

struct TestState;

impl<'a, 'b> SimpleState<'a, 'b> for TestState {
    fn on_start(&mut self, data: StateData<GameData>) {
        for _ in 0..2_500 {
            data.world.create_entity().build();
        }
    }
}

fn main() -> amethyst::Result<()> {
    let editor_sync_bundle = SyncEditorBundle::new();
    let game_data = GameDataBuilder::default()
        .with_bundle(editor_sync_bundle)?;
    let mut game = Application::build(".", TestState)?
        .build(game_data)?;
    game.run();
    Ok(())
}
@randomPoison randomPoison added the bug Something isn't working label Sep 29, 2018
@randomPoison
Copy link
Member

Ah, thanks for finding this! I'm pretty sure I've run into this issue and hadn't yet figured out where the crash was coming from. We could fix this with UDP pretty easily by splitting the message across multiple packets, though this also makes me want to investigate using proper IPC communication or maybe even TCP.

Ideally ipc-channel would be the way to go, but Windows support is still a work in progress. So in the meantime I'd probably suggest we go with TCP, since I think that will be the easiest for editor implementations to support.

@randomPoison
Copy link
Member

This would also be a good thing to add a regression test for!

@randomPoison
Copy link
Member

Thinking about it a bit more, it's not going to be so trivial to convert to TCP 🤔 With TCP we'd have to manually manage connection states, send heartbeats, and track disconnections after a timeout. This is stuff we'll want to do eventually, but I'm not sure it's worth putting in that kind of effort for now. Instead I'm going to doing a pretty simple implementation of splitting the state message across multiple UDP packets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants