spicylobstergames / shotcaller-minigene

A moddable RTS/MOBA game made with bracket-lib and minigene.
https://www.notion.so/erlendsh/Shotcaller-7374d2b2819c42ccb40f01dc7089d419
Other
152 stars 12 forks source link

Movement system refactor #54

Closed Winsalot closed 3 years ago

Winsalot commented 3 years ago

The way I see movement system at the moment, it has 2 problems:

First. Currently there are 3 systems for movement: leader1_simple_movement.rs, leader2_simple_movement.rs and simple_movement.rs. 3 systems for essentially the same thing is little bit too much.

Second problem, which is more relevant for me personally, is the whole architecture and logic of the movement as a whole. Currently it looks like this:

  1. System checks if unit has enough action points to move. If yes:
  2. Runs the logic to find a target destination.
  3. Adds AiDestination component.
  4. On minigene side, this AiDestination component is used to compute AiPath component.
  5. On minigene side unit gets moved to the next point on AiPath.
  6. If on step 1. unit had not enough action points to move, then both AiPath, and AiDestination components get removed from entity (thus preventing movement).

Now this implementation has potential performance issues, because pathfinding gets computed every time a unit moves a single step. But I see there a much bigger "architectural" issue that causes problems with code scalability. In my view movement system should would something like this:

  1. Find where unit needs to go. Set AiDesination.
  2. Run pathfinding (don't run if it was already computed and AiDestination didn't change since last frame)
  3. If unit has enough action points, move it 1 point forward on AIpath.
  4. Repeat steps 2 & 3. (step 2 to take into account moving targets)

The way I see it now is that steps 1 and half of step 3 (checking for action points) are now in a single system. Another half of step 3 (actual change of position) is in another system in minigene.

I also dislike the constant adding/removing of AiPath, and AiDestination components. I think it would be more consistent if all units had these components for all times, and if they don't have anywhere to move, then their destination would simply be their current position.

Now to implement this refactor, we would have to rewrite the architecture, and then implement all three leader1_simple_movement.rs, leader2_simple_movement.rs and simple_movement.rs into a single system that sets destination based on various conditions.

Winsalot commented 3 years ago

PRs created: https://github.com/amethyst/shotcaller/pull/55 and https://github.com/jojolepro/minigene/pull/3