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

Initial combat system #21

Merged
merged 1 commit into from Apr 30, 2019

Conversation

Projects
None yet
2 participants
@marot
Copy link
Collaborator

commented Apr 27, 2019

My take on the combat system. This implements #9 and #8, since they are closely related.

The components should be self-explanatory, except for HasFaction and FactionEnemies. They are used to determine which entities can attack each other. I decided to implement it that way to avoid switching on the tags (HerbivoreTag, CarnivoreTag, PlantTag). Another idea I had was to use an enum, but that would be closed and thus unflexible.

There are three systems for attacking. One to determine if an attack happens after a collision FindAttackSystem, one to perform the attack PerformDefaultAttackSystem. They communicate with each other using events. The last one is CooldownSystem to remove Cooldown components after the cooldown finished.

let mut to_remove = Vec::new();

for (mut cooldown, entity) in (&mut cooldowns, &*entities).join() {
match cooldown.time_left.checked_sub(time.delta_time()) {

This comment has been minimized.

Copy link
@khskarl

khskarl Apr 30, 2019

Member

I would only change delta_time to fixed_time, delta_time will make simulations depend on the frame-rate stability, making the simulation non-deterministic, which makes debugging harder and we lose a possible feature.

Suggested change
match cooldown.time_left.checked_sub(time.delta_time()) {
match cooldown.time_left.checked_sub(time.fixed_time()) {

This comment has been minimized.

Copy link
@marot

marot Apr 30, 2019

Author Collaborator

@khskarl I think fixed_time is not affected by time_scale. I think it would also be problematic because delta_time would reflect the time between two computations, while fixed_time is constant, with an unlimited frame rate the results would change a lot.

I'll merge it for now and maybe we can revisit this topic when we decide to have multiple systems for different domains (e.g. one system group for simulations and one for effects)

@khskarl

This comment has been minimized.

Copy link
Member

commented Apr 30, 2019

@marot
I will leave you to commit the suggestion if you agree and merge the PR :).

@marot marot merged commit 13966d7 into amethyst:master Apr 30, 2019

This was referenced Apr 30, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.