Refactor System Marker Components and Rename Creeps #53
Conversation
owen-mccormick
changed the title
There was a problem hiding this comment.
Nice PR!
However, I have some doubts on the merging of the two components into an enum. I'm wondering if it would be possible to split the movement and attack components into multiple, smaller components and systems. For example, if we look at the leader2_simple_movement_system, we see that a good part of the code is duplicated.
We could split those into a Caught System and Movement system. Furthermore, I can barely see a difference between the leader's movement system and the pawn's movement system.
The two difference I see are:
- different targeting ranges (we can use a variable in the component for this, and a second, more specific system to override the default movement system's behavior after it has run).
- Only one of them takes action points into account. I assume this is a bug? I think it could be worth looking into a way where we could separate the movement systems (setting the destination target) and handling action points for movement (consuming points if a movement target is set, and removing it for this frame if not enough action points are available.)
Let me know what you think!
| if let Some((target, _)) = closest { | ||
| let damage = stat.unwrap().stats.get(&Stats::Attack).unwrap().value; | ||
| v.push((e.unwrap().clone(), target.clone(), damage)); | ||
| if let ProximityAttackSystems::Leader1ProximityAttack(radius) = proximity.unwrap() { |
There was a problem hiding this comment.
The fact that the component/enum contains the word Systems is quite confusing. Could we change it to something else?
Description
This PR replaces separate marker components for different movement and proximity attack systems with individual enum components. Ultimately, this will help reduce the number of components that certain systems need access to and mitigate the issues caused by the argument limit on systems. A minor naming change from creeps to pawns was also performed.