-
Notifications
You must be signed in to change notification settings - Fork 33
[WIP] A basic day-night cycle that handles music swapping #80
Conversation
There was a problem hiding this 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.
@marot yes absolutely! It'd be very useful for me |
|
||
use crate::Music; | ||
|
||
pub struct DayNightCycle{ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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{ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
have left a comment on #81 (comment) about how the audio system could be done |
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! |
What this pull request contains:
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!