Lagging issues with bevy

[bayou-brogrammer]

Have you noticed a weird lagging issue when moving around in the bevy port? It seems to be linked to the camera movement where the monsters are basically being rendered twice. Here is a gif of it happening Lagging

[64kramsystem]

Hi there!

Thanks for the report. For mysterious reasons, I've missed this (GitHub) issue - I've found it by accident 😓. Apologies!

I'll check this over the next couple of days, and get back 😄

[64kramsystem]

Damn. I can reproduce it 🤬. I'll debug it; thanks again for the report 😄

[64kramsystem]

Interesting. This is an issue (I'd classify it as bug) also of the source project 🤯.

I'm not sure what's the exact cause. If you stand still and press keys that don't cause movement, the enemies will move, but without this flickering. So it's related, in some way, to the map moving.

It's present from step 07.02, where enemies turn-based movement is introduced.

[64kramsystem]

Bug cause here:

map_render at 9,35
entity_render, with offset: 9,35

player_input                       // Goes left
player moving camera               // Changes stage, but Bevy doesn't follow up on the next stage, and moves to next frame

map_render at 9,34                 // Next frame
entity_render, with offset: 9,34   // Enemy drawn with old position in new camera position
collisions                         // Again: changes stage, but Bevy doesn't follow up on the next stage, and moves to next frame

map_render at 9,34                 // Next frame
entity_render, with offset: 9,34
random_move                        // Enemy moves only now
                                   // changes stage, but Bevy etc.etc.

map_render at 9,34
entity_render, with offset: 9,34   // Enemy drawn with new position in new camera location
player_input

it's curious that it also affects also the source project, since this the 1-frame-lag problem, which doesn't affect Legion (which can perform commands flushing).

@lecoqjacob could you confirm that the probem reproduces on the source project for you as well (e.g. from $repo_path/source_projects/rusty_roguelike/07_TurnBasedGames_02_turnbased)?

[64kramsystem]

Summary: It seems that iyes_loopless ("YL") decides which system (condition) sets will run at the beginning of the frame, so that if the state is changed during one stage, it (YL) doesn't update the plan, and moves Bevy to the next stage. If it's not this, it's something very close.

If this is correct, this is a big hairy problem - it implies that one can't alter the current flow (in terms of which system sets to run) within one frame.

[bayou-brogrammer]

State transitions have always been the downfall of bevy and one of the reasons they are currently rebuilding the entire state management inside of bevy. This issue is one thing preventing me from actually utilizing bevy to make a rogue like. Everything is perfect until you introduce camera movement into the equation

[bayou-brogrammer]

@64kramsystem and yes I ran into the issue in the source project first which caused me to abandon the project (hands on rust) and then I found your port and was super excited to try it. Whenever I got to the camera portion, it still happened.

What would cause this issue even in the source project with legion?

[64kramsystem]

What would cause this issue even in the source project with legion?

Thanks for checking! That's a good question. I haven't had a look at the source project; the bug in the port alone caused me very severe hair loss for the last couple of hours.

I think the bug in the source project should be filed to the source project 😄, since I have the impression that are two different issues, only accidentally happening on two different project (but that's a guess!).

At a very quick check, if you look at the schedules:

pub fn build_input_scheduler() -> Schedule {
    Schedule::builder()
        .add_system(player_input::player_input_system()) // camera position changed here
        .flush()                                                                                // and flushed here
        .add_system(map_render::map_render_system())
        .add_system(entity_render::entity_render_system()) // enemies are drawn in the same position
        .build()
}

pub fn build_player_scheduler() -> Schedule {
    Schedule::builder()
        .add_system(collisions::collisions_system())
        .flush()
        .add_system(map_render::map_render_system())
        .add_system(entity_render::entity_render_system()) // enemies drawn in the same position again
        .add_system(end_turn::end_turn_system())
        .build()
}

pub fn build_monster_scheduler() -> Schedule {
    Schedule::builder()
        .add_system(random_move::random_move_system()) // now enemies finally move
        .flush()
        .add_system(collisions::collisions_system())
        .flush()
        .add_system(map_render::map_render_system())
        .add_system(entity_render::entity_render_system()) // and here they're rendered in the new position
        .add_system(end_turn::end_turn_system())
        .build()
}

you'll see that after the camera moves, the enemies are rendered for at least two frames with the new camera position, but before actually moving.

it's tricky to solve this in the source project (or at least, not trivial), since it renders one schedule per frame.

This is different from the problem that the port suffers, which I describe here.

I have the suspicion that this issue may not be fixable in the port (!). If one can't change state (with effect) mid-frame, then the port will need to render one schedule per frame... which will make the port suffer the same problem of the source.

[64kramsystem]

He. The source project doesn't accept issues 🤷.

[64kramsystem]

Just for reference: this can be fixed in the port, by moving the conditionals from the system set abstraction (level) down to the systems level, via simple if/else conditionals.

This is butt-ugly, though, as each conditional will need to be duplicated across all the systems of a system set, and additionally, all the systems will run, and will also be part of the scheduler graph.

[bayou-brogrammer]

Yeah I actually tried opening an issue when this arose but didn't have any luck. I thought about reaching out to bracket to see if he had time to check, but he is so busy with life I figured I shouldn't.

Do you plan to put the fix in place for your port or because of the ugliness you rather not?

[64kramsystem]

It seems that there is a clean solution 😅 A teammate pointed me to a discussion in a Bevy's channel. Using states may seem intuitively the correct solution (at least, to a non-knowledgeable user (like me)), but it isn't - one needs to use a Resource for that (!). I'll check how this works out and update on this issue 😄

[64kramsystem]

It works. Phew 😅

Have a look here.

I'm going to take some time to apply the fix to the project, because due to the nature of the Rusty Roguelike port project, I will need to apply this change to approximately 20 workspaces, which may be simple... or not.

In the meanwhile, you can just apply this commit.

[bayou-brogrammer]

Amazing news! Also, from what I am reading through the discussions you posted, would Bevy states and scheduler be able to solve the issue iyes_loopless has?

[64kramsystem]

Amazing news! Also, from what I am reading through the discussions you posted, would Bevy states and scheduler be able to solve the issue iyes_loopless has?

AFAIK Bevy's state management is broken. Based on iyes_loopless' author discussion, and what you can see in the branch, using resources instead of state, solves the problem. It's not an issue with iyes_loopless in the sense of a bug, rather, a lack of documentation that makes it not obvious what to use.

[64kramsystem]

Closing; this was closed by #112. If you feel like contributing, backporting the fix to the pre-15.02 steps would be great! 😄

[bayou-brogrammer]

With the stageless rfc coming ever closer, this stage method to bypass the command flushing will need to be reevaluated. But they plan to add explicit command flushing by then

[bayou-brogrammer]

I'll create a pr either today or tomorrow to help with the backfill