Skip to content
This repository was archived by the owner on Oct 25, 2021. It is now read-only.

Conversation

QbieShay
Copy link

What this pull request contains:

  • a daynight system
  • an extension of the audio system to support named soundtracks

Limitation: the current audio system in evoli does not allow to force a change of music, and I'm a bit stuck on making another audio system because I'm very new to Rust and Amethyst.

Feedback, suggestions, help would be highly apprecciated!

Copy link
Contributor

@marotili marotili left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general.

Unfortunately, I don't know how to solve the switching audio problem. Maybe if you ask in the #audio channel on Discord somebody can help you :)

I would have some notes regarding the separation of concerns in the DayNightCycle system, if you are interested in hearing them.

@QbieShay
Copy link
Author

@marot yes absolutely! It'd be very useful for me


use crate::Music;

pub struct DayNightCycle{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If data is stored in a system, it can only be read or changed by the system or by re-registering the system in the dispatcher. If we add a UI displaying the elapsed cycle time, or if we want to add debug utilities to change the cycle time, we have to change the implementation.

So imo we should not store data in systems, to begin with. Two possible solutions are to store the data in one or multiple components and attach them to an entity. The system would only store the entity so that it can access the components. The second solution would be to store the data as a Resource.

Personally, I dislike storing data in resources because they are less flexible than components.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is more flexible about entities/component compared to resources?
The way I understood it is that Resources are for "singletons" while entities are for things that have multiple instances.

pub night_music_name: String,
}

impl<'s> System<'s> for DayNightCycle{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make sense to split this up into two systems. One that determines if it is day or night and one that switches the music based on that.
The system switching the music could be event driven. The system determining if it is day or night would dispatch an event with the CycleState as the payload.
The advantage of splitting that is that you'd be able to enable/disable individual systems for faster prototyping. In addition, other effects of the day/night cycle don't need to modify existing systems but can be built on top.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks! I'll look into events :)

@azriel91
Copy link
Member

have left a comment on #81 (comment) about how the audio system could be done

@khskarl khskarl changed the title WIP: A basic daynight cycle that handles music swapping [WIP] A basic day-night cycle that handles music swapping Jul 6, 2019
@QbieShay
Copy link
Author

Hello everyone!

Thanks a lot for all your input below this issue and the audio one. Unfortunately I've lacked time/energy to follow up on this, so I'm closing this PR.

Good luck with the project and thank you all for being so kind and explaining me how to improve the code!

@QbieShay QbieShay closed this Jul 15, 2019
@erlend-sh
Copy link
Member

This is now being revisited in #102

Thanks for looking into this and helping nudge it along @QbieShay 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants