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

Reimplement Runs, Levels, and Phases #89

Merged
merged 27 commits into from
Sep 1, 2023

Conversation

cdsupina
Copy link
Contributor

@cdsupina cdsupina commented Aug 21, 2023

  • Runs are made up of levels, levels are made up of phases
    • Phases determine specific things that happen at a given point in the level of a run (Boss Fight, Mobs Spawning, Break, etc)
    • Levels group phases and have an ongoing objective that persists through its phases (Defense)
    • Runs are made up of levels, completing all the levels in a run results in a victory
  • Added a premade run defined in a run file with 3 identical levels
  • Created event for playing sounds effects
  • Created event for changing background music
  • Created event for ending a run

@@ -493,7 +493,7 @@ pub fn update_player1_ui(
}

for mut style_component in player1_ui_queries.p6().iter_mut() {
if let Some(level) = &run_resource.level {
if let Some(level) = &run_resource.current_level {
Copy link
Contributor

Choose a reason for hiding this comment

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

I hope you used an IDE/right-click->refactor->rename for this :)

src/run/level.rs Outdated
pub phase_timer: Option<Timer>,
/// Tracks time between spawns
pub spawn_timer: Option<Timer>,
pub completed_phases: VecDeque<LevelPhase>,
Copy link
Contributor

@varoonp123 varoonp123 Aug 21, 2023

Choose a reason for hiding this comment

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

Maybe pub type LevelPhases = VecDeque<LevelPhase>; to save some typing and ease changing the implementation (atleast as much as others parts of the code base rely on common traits/interfaces).

I have seen rustc compile these type aliases away on godbolt, so there's probably not a runtime cost.

@varoonp123
Copy link
Contributor

I resolved the merge conflicts. You know, you can do

mob_query.contains(*mob_entity)

and avoid this for loop.

@varoonp123 varoonp123 closed this Aug 22, 2023
@varoonp123 varoonp123 reopened this Aug 22, 2023
@varoonp123
Copy link
Contributor

varoonp123 commented Aug 22, 2023

Step on up, fam, 'cause we're breakin' free from that old colonial code grind. A new dawn's risin', and this era? It's shackled to nothin' but top-tier code. Listen up, 'fore we ride that CI wave, there's a game-changer in town. I'm talkin' 'bout rustlin' that Rust project, givin' them warnings a new name: compile errors. You best believe, it's like breakin' chains 'fore they lock up – cargo rustc --all-features -- -D warnings – that's the spell we're castin', turnin' 'em warnings into errors right out the gate.

Hold tight, 'cause here's the deal: I'm only pullin' up a chair for code that ain't just talkin' the talk. It's gotta strut through that CI gauntlet without so much as a hiccup, and when it comes to mixin' up that Rust stew, them warnings best be as quiet as a hush-hush revolution. We ain't got time for noise, we're on the hunt for the real deal, like uncoverin' hidden treasures buried deep. So, remember this, partners: if it ain't passin' that CI litmus test and ain't ridin' the compile trail without raisin' no warnings ruckus, it ain't gettin' the spotlight in my code rodeo – 'cause we're breakin' chains, not just rebrandin' 'em.

    Compiling thetawave v0.1.1 (/home/runner/work/thetawave/thetawave)
error: method `generate_random` is never used
  --> src/run/mod.rs:99:12
   |
98 | impl RunResource {
   | ---------------- method in this implementation
99 |     pub fn generate_random(&mut self) {}
   |            ^^^^^^^^^^^^^^^
   |
   = note: `-D dead-code` implied by `-D warnings`

error: function `run_reset_system` is never used
   --> src/run/mod.rs:252:4
    |
252 | fn run_reset_system(mut run_resource: ResMut<RunResource>) {
    |    ^^^^^^^^^^^^^^^^

error: could not compile `thetawave` (bin "thetawave") due to 2 previous errors
Error: Process completed with exit code 101.

@@ -12,6 +12,25 @@ pub enum BGMusicType {
Main,
}

#[derive(Deserialize, Debug, Hash, PartialEq, Eq, Clone, Display)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this and PlaySoundEffectEvent to thetawave_interface::audio. Now there is a clear way for other plugins to play sound and allow this 1 module/compilation unit to deal with all audio assets, platform-specific audio libs/crap, etc.

@cdsupina cdsupina marked this pull request as ready for review August 29, 2023 15:31
src/run/mod.rs Outdated
PlayersDestroyed,
DefenseDestroyed,
#[derive(Resource, Default, Debug)]
pub struct RunResource {
Copy link
Contributor

@varoonp123 varoonp123 Aug 29, 2023

Choose a reason for hiding this comment

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

rename to CurrentRunProgress or CurrentRunState just to emphasize that this thing is a state-machine/game-level mutable global of sorts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm renaming it to CurrentRunProgressResource because I want it to still be marked as a Resource.

@varoonp123
Copy link
Contributor

Screenshot from 2023-08-30 09-59-23

The fire rate gets a bit wack.

/// Loop from a specific time in the track, None will not loop the track
pub loop_from: Option<f64>,
/// Fade in the music cycling in (bg_music_type)
pub fade_in_tween: Option<AudioTween>,
Copy link
Contributor

Choose a reason for hiding this comment

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

If we only use this linear tween, then use a Duration so that we dont need to pull in kira/audio stuff just for the interface. Nice for testing, but not required.

Copy link
Contributor

@varoonp123 varoonp123 left a comment

Choose a reason for hiding this comment

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

Ill help write a test on the state transitions on RunResource (which seems like the meat of this PR) before we merge.

@varoonp123
Copy link
Contributor

Looks good and plays fine. Ill try getting this test to work. And if I cant get it by tomorrow evening, lets just merge. Norway won't invade.

Copy link
Contributor

@varoonp123 varoonp123 left a comment

Choose a reason for hiding this comment

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

It's fine for now and plays well. I added a couple of (regression) tests of the core defense health objective and will sprinkle in more over time. I was about to add more but idk how much I like SortedCollisionEvent enough to hoist it to the interface (IOW idk how stable that enum is)...

@varoonp123 varoonp123 merged commit d6c025d into thetawavegame:main Sep 1, 2023
5 checks passed
@cdsupina cdsupina deleted the features/BetterLevels branch September 3, 2023 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants