refactor: replace immutable Hidden component with mutable Visibility component

This commit is contained in:
Ryan Walters
2025-09-10 00:45:16 -05:00
parent cb691b0907
commit 5563b64044
5 changed files with 141 additions and 85 deletions

View File

@@ -16,10 +16,10 @@ use crate::systems::{
hud_render_system, item_system, linear_render_system, player_life_sprite_system, present_system, profile,
time_to_live_system, touch_ui_render_system, AudioEvent, AudioResource, AudioState, BackbufferResource, Blinking,
BufferedDirection, Collider, DebugState, DebugTextureResource, DeltaTime, DirectionalAnimation, EntityType, Frozen,
GameStage, Ghost, GhostAnimation, GhostAnimations, GhostBundle, GhostCollider, GhostState, GlobalState, Hidden, ItemBundle,
GameStage, Ghost, GhostAnimation, GhostAnimations, GhostBundle, GhostCollider, GhostState, GlobalState, ItemBundle,
ItemCollider, LastAnimationState, LinearAnimation, MapTextureResource, MovementModifiers, NodeId, PacmanCollider,
PlayerAnimation, PlayerBundle, PlayerControlled, PlayerDeathAnimation, PlayerLives, Position, RenderDirty, Renderable,
ScoreResource, StartupSequence, SystemId, SystemTimings, Timing, TouchState, Velocity,
ScoreResource, StartupSequence, SystemId, SystemTimings, Timing, TouchState, Velocity, Visibility,
};
use crate::texture::animated::{DirectionalTiles, TileSequence};
@@ -148,7 +148,7 @@ impl Game {
Self::configure_schedule(&mut schedule);
debug!("Spawning player entity");
world.spawn(player_bundle).insert((Frozen, Hidden));
world.spawn(player_bundle).insert((Frozen, Visibility::hidden()));
info!("Spawning game entities");
Self::spawn_ghosts(&mut world)?;
@@ -601,7 +601,7 @@ impl Game {
}
};
let entity = world.spawn(ghost).insert((Frozen, Hidden)).id();
let entity = world.spawn(ghost).insert((Frozen, Visibility::hidden())).id();
trace!(ghost = ?ghost_type, entity = ?entity, start_node, "Spawned ghost entity");
}

View File

@@ -1,11 +1,10 @@
use bevy_ecs::{
component::Component,
entity::Entity,
query::{Has, With},
system::{Commands, Query, Res},
system::{Query, Res},
};
use crate::systems::{DeltaTime, Frozen, Hidden, Renderable};
use crate::systems::{DeltaTime, Frozen, Renderable, Visibility};
#[derive(Component, Debug)]
pub struct Blinking {
@@ -28,18 +27,11 @@ impl Blinking {
/// accumulating ticks and toggling visibility when the specified interval is reached.
/// Uses integer arithmetic for deterministic behavior.
#[allow(clippy::type_complexity)]
pub fn blinking_system(
mut commands: Commands,
time: Res<DeltaTime>,
mut query: Query<(Entity, &mut Blinking, Has<Hidden>, Has<Frozen>), With<Renderable>>,
) {
for (entity, mut blinking, hidden, frozen) in query.iter_mut() {
// If the entity is frozen, blinking is disabled and the entity is unhidden (if it was hidden)
pub fn blinking_system(time: Res<DeltaTime>, mut query: Query<(&mut Blinking, &mut Visibility, Has<Frozen>), With<Renderable>>) {
for (mut blinking, mut visibility, frozen) in query.iter_mut() {
// If the entity is frozen, blinking is disabled and the entity is made visible
if frozen {
if hidden {
commands.entity(entity).remove::<Hidden>();
}
visibility.show();
continue;
}
@@ -49,11 +41,7 @@ pub fn blinking_system(
// Handle zero interval case (immediate toggling)
if blinking.interval_ticks == 0 {
if time.ticks > 0 {
if hidden {
commands.entity(entity).remove::<Hidden>();
} else {
commands.entity(entity).insert(Hidden);
}
visibility.toggle();
}
continue;
}
@@ -69,14 +57,10 @@ pub fn blinking_system(
// Update the timer to the remainder after complete intervals
blinking.tick_timer %= blinking.interval_ticks;
// Toggle the Hidden component for each complete interval
// Toggle the visibility for each complete interval
// Since toggling twice is a no-op, we only need to toggle if the count is odd
if complete_intervals % 2 == 1 {
if hidden {
commands.entity(entity).remove::<Hidden>();
} else {
commands.entity(entity).insert(Hidden);
}
visibility.toggle();
}
}
}

View File

@@ -8,7 +8,7 @@ use crate::texture::sprite::{AtlasTile, SpriteAtlas};
use bevy_ecs::component::Component;
use bevy_ecs::entity::Entity;
use bevy_ecs::event::EventWriter;
use bevy_ecs::query::{Changed, Or, With, Without};
use bevy_ecs::query::{Changed, Or, With};
use bevy_ecs::removal_detection::RemovedComponents;
use bevy_ecs::resource::Resource;
use bevy_ecs::system::{NonSendMut, Query, Res, ResMut};
@@ -33,6 +33,53 @@ pub struct RenderDirty(pub bool);
#[derive(Component)]
pub struct Hidden;
/// A component that controls entity visibility in the render system.
///
/// Entities without this component are considered visible by default.
/// This allows for efficient rendering where only entities that need
/// visibility control have this component.
#[derive(Component, Debug, Clone, Copy, PartialEq, Eq)]
pub struct Visibility(pub bool);
impl Default for Visibility {
fn default() -> Self {
Self(true) // Default to visible
}
}
impl Visibility {
/// Creates a visible Visibility component
pub fn visible() -> Self {
Self(true)
}
/// Creates a hidden Visibility component
pub fn hidden() -> Self {
Self(false)
}
/// Returns true if the entity is visible
pub fn is_visible(&self) -> bool {
self.0
}
/// Returns true if the entity is hidden
#[allow(dead_code)] // Used in tests
pub fn is_hidden(&self) -> bool {
!self.0
}
/// Makes the entity visible
pub fn show(&mut self) {
self.0 = true;
}
/// Toggles the visibility state
pub fn toggle(&mut self) {
self.0 = !self.0;
}
}
/// Enum to identify which texture is being rendered to in the combined render system
#[derive(Debug, Clone, Copy)]
enum RenderTarget {
@@ -43,15 +90,10 @@ enum RenderTarget {
#[allow(clippy::type_complexity)]
pub fn dirty_render_system(
mut dirty: ResMut<RenderDirty>,
changed: Query<(), Or<(Changed<Renderable>, Changed<Position>)>>,
removed_hidden: RemovedComponents<Hidden>,
changed: Query<(), Or<(Changed<Renderable>, Changed<Position>, Changed<Visibility>)>>,
removed_renderables: RemovedComponents<Renderable>,
) {
let changed_count = changed.iter().count();
let removed_hidden_count = removed_hidden.len();
let removed_renderables_count = removed_renderables.len();
if changed_count > 0 || removed_hidden_count > 0 || removed_renderables_count > 0 {
if changed.iter().count() > 0 || !removed_renderables.is_empty() {
dirty.0 = true;
}
}
@@ -77,8 +119,14 @@ pub fn render_system(
map: &Res<Map>,
dirty: &Res<RenderDirty>,
renderables: &Query<
(Entity, &Renderable, Option<&Position>, Option<&PixelPosition>),
(Without<Hidden>, Or<(With<Position>, With<PixelPosition>)>),
(
Entity,
&Renderable,
Option<&Position>,
Option<&PixelPosition>,
Option<&Visibility>,
),
Or<(With<Position>, With<PixelPosition>)>,
>,
errors: &mut EventWriter<GameError>,
) {
@@ -95,14 +143,17 @@ pub fn render_system(
errors.write(TextureError::RenderFailed(e.to_string()).into());
}
// Render all entities to the backbuffer
for (_entity, renderable, position, pixel_position) in renderables
// Collect and filter visible entities, then sort by layer
let mut visible_entities: Vec<_> = renderables
.iter()
.sort_by_key::<(Entity, &Renderable, Option<&Position>, Option<&PixelPosition>), _>(|(_, renderable, _, _)| {
renderable.layer
})
.rev()
{
.filter(|(_, _, _, _, visibility)| visibility.map(|v| v.is_visible()).unwrap_or(true))
.collect();
visible_entities.sort_by_key(|(_, renderable, _, _, _)| renderable.layer);
visible_entities.reverse();
// Render all visible entities to the backbuffer
for (_entity, renderable, position, pixel_position, _visibility) in visible_entities {
let pos = if let Some(position) = position {
position.get_pixel_position(&map.graph)
} else {
@@ -150,8 +201,14 @@ pub fn combined_render_system(
map: Res<Map>,
dirty: Res<RenderDirty>,
renderables: Query<
(Entity, &Renderable, Option<&Position>, Option<&PixelPosition>),
(Without<Hidden>, Or<(With<Position>, With<PixelPosition>)>),
(
Entity,
&Renderable,
Option<&Position>,
Option<&PixelPosition>,
Option<&Visibility>,
),
Or<(With<Position>, With<PixelPosition>)>,
>,
colliders: Query<(&Collider, &Position)>,
cursor: Res<CursorPosition>,

View File

@@ -6,8 +6,8 @@ use crate::systems::SpawnTrigger;
use crate::{
map::builder::Map,
systems::{
AudioEvent, Blinking, DirectionalAnimation, Dying, Eaten, Frozen, Ghost, GhostCollider, GhostState, Hidden,
LinearAnimation, Looping, NodeId, PlayerControlled, Position,
AudioEvent, Blinking, DirectionalAnimation, Dying, Eaten, Frozen, Ghost, GhostCollider, GhostState, LinearAnimation,
Looping, NodeId, PlayerControlled, Position, Visibility,
},
};
use bevy_ecs::{
@@ -223,8 +223,8 @@ pub fn stage_system(
}
// Hide the player & eaten ghost
commands.entity(player.0).insert(Hidden);
commands.entity(ghost_entity).insert(Hidden);
commands.entity(player.0).insert(Visibility::hidden());
commands.entity(ghost_entity).insert(Visibility::hidden());
// Spawn bonus points entity at Pac-Man's position
commands.trigger(SpawnTrigger::Bonus {
@@ -236,9 +236,9 @@ pub fn stage_system(
}
(GameStage::GhostEatenPause { ghost_entity, .. }, GameStage::Playing) => {
// Unfreeze and reveal the player & all ghosts
commands.entity(player.0).remove::<(Frozen, Hidden)>();
commands.entity(player.0).remove::<Frozen>().insert(Visibility::visible());
for (entity, _, _) in ghost_query.iter_mut() {
commands.entity(entity).remove::<(Frozen, Hidden)>();
commands.entity(entity).remove::<Frozen>().insert(Visibility::visible());
}
// Reveal the eaten ghost and switch it to Eyes state
@@ -254,7 +254,7 @@ pub fn stage_system(
(GameStage::PlayerDying(DyingSequence::Frozen { .. }), GameStage::PlayerDying(DyingSequence::Animating { .. })) => {
// Hide the ghosts
for (entity, _, _) in ghost_query.iter_mut() {
commands.entity(entity).insert(Hidden);
commands.entity(entity).insert(Visibility::hidden());
}
// Start Pac-Man's death animation
@@ -265,7 +265,7 @@ pub fn stage_system(
}
(GameStage::PlayerDying(DyingSequence::Animating { .. }), GameStage::PlayerDying(DyingSequence::Hidden { .. })) => {
// Hide the player
commands.entity(player.0).insert(Hidden);
commands.entity(player.0).insert(Visibility::hidden());
}
(_, GameStage::LevelRestarting) => {
let (player_entity, mut pos) = player.into_inner();
@@ -275,7 +275,7 @@ pub fn stage_system(
// Freeze the blinking, force them to be visible (if they were hidden by blinking)
for entity in blinking_query.iter_mut() {
commands.entity(entity).insert(Frozen).remove::<Hidden>();
commands.entity(entity).insert(Frozen).insert(Visibility::visible());
}
// Reset the player animation
@@ -296,15 +296,15 @@ pub fn stage_system(
};
commands
.entity(ghost_entity)
.remove::<(Frozen, Hidden, Eaten)>()
.insert(GhostState::Normal);
.remove::<(Frozen, Eaten)>()
.insert((Visibility::visible(), GhostState::Normal));
}
}
(_, GameStage::Starting(StartupSequence::CharactersVisible { .. })) => {
// Unhide the player & ghosts
commands.entity(player.0).remove::<Hidden>();
commands.entity(player.0).insert(Visibility::visible());
for (entity, _, _) in ghost_query.iter_mut() {
commands.entity(entity).remove::<Hidden>();
commands.entity(entity).insert(Visibility::visible());
}
}
(GameStage::Starting(StartupSequence::CharactersVisible { .. }), GameStage::Playing) => {

View File

@@ -1,5 +1,5 @@
use bevy_ecs::{entity::Entity, system::RunSystemOnce, world::World};
use pacman::systems::{blinking_system, Blinking, DeltaTime, Frozen, Hidden, Renderable};
use pacman::systems::{blinking_system, Blinking, DeltaTime, Frozen, Renderable, Visibility};
use speculoos::prelude::*;
mod common;
@@ -20,11 +20,12 @@ fn spawn_blinking_entity(world: &mut World, interval_ticks: u32) -> Entity {
sprite: common::mock_atlas_tile(1),
layer: 0,
},
Visibility::visible(),
))
.id()
}
/// Spawns a test entity with blinking, renderable, and hidden components
/// Spawns a test entity with blinking, renderable, and hidden visibility
fn spawn_hidden_blinking_entity(world: &mut World, interval_ticks: u32) -> Entity {
world
.spawn((
@@ -33,7 +34,7 @@ fn spawn_hidden_blinking_entity(world: &mut World, interval_ticks: u32) -> Entit
sprite: common::mock_atlas_tile(1),
layer: 0,
},
Hidden,
Visibility::hidden(),
))
.id()
}
@@ -47,12 +48,13 @@ fn spawn_frozen_blinking_entity(world: &mut World, interval_ticks: u32) -> Entit
sprite: common::mock_atlas_tile(1),
layer: 0,
},
Visibility::visible(),
Frozen,
))
.id()
}
/// Spawns a test entity with blinking, renderable, hidden, and frozen components
/// Spawns a test entity with blinking, renderable, hidden visibility, and frozen components
fn spawn_frozen_hidden_blinking_entity(world: &mut World, interval_ticks: u32) -> Entity {
world
.spawn((
@@ -61,7 +63,7 @@ fn spawn_frozen_hidden_blinking_entity(world: &mut World, interval_ticks: u32) -
sprite: common::mock_atlas_tile(1),
layer: 0,
},
Hidden,
Visibility::hidden(),
Frozen,
))
.id()
@@ -73,9 +75,22 @@ fn run_blinking_system(world: &mut World, delta_ticks: u32) {
world.run_system_once(blinking_system).unwrap();
}
/// Checks if an entity has the Hidden component
fn has_hidden_component(world: &World, entity: Entity) -> bool {
world.entity(entity).contains::<Hidden>()
/// Checks if an entity is visible
fn is_entity_visible(world: &World, entity: Entity) -> bool {
world
.entity(entity)
.get::<Visibility>()
.map(|v| v.is_visible())
.unwrap_or(true) // Default to visible if no Visibility component
}
/// Checks if an entity is hidden
fn is_entity_hidden(world: &World, entity: Entity) -> bool {
world
.entity(entity)
.get::<Visibility>()
.map(|v| v.is_hidden())
.unwrap_or(false) // Default to visible if no Visibility component
}
/// Checks if an entity has the Frozen component
@@ -100,7 +115,7 @@ fn test_blinking_system_normal_interval_no_toggle() {
run_blinking_system(&mut world, 3);
// Entity should not be hidden yet
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
// Check that timer was updated
let blinking = world.entity(entity).get::<Blinking>().unwrap();
@@ -116,7 +131,7 @@ fn test_blinking_system_normal_interval_first_toggle() {
run_blinking_system(&mut world, 5);
// Entity should now be hidden
assert_that(&has_hidden_component(&world, entity)).is_true();
assert_that(&is_entity_hidden(&world, entity)).is_true();
// Check that timer was reset
let blinking = world.entity(entity).get::<Blinking>().unwrap();
@@ -130,11 +145,11 @@ fn test_blinking_system_normal_interval_second_toggle() {
// First toggle: 5 ticks
run_blinking_system(&mut world, 5);
assert_that(&has_hidden_component(&world, entity)).is_true();
assert_that(&is_entity_hidden(&world, entity)).is_true();
// Second toggle: another 5 ticks
run_blinking_system(&mut world, 5);
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
}
#[test]
@@ -146,7 +161,7 @@ fn test_blinking_system_normal_interval_multiple_intervals() {
run_blinking_system(&mut world, 7);
// Should toggle twice (even number), so back to original state (not hidden)
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
// Check that timer was updated to remainder
let blinking = world.entity(entity).get::<Blinking>().unwrap();
@@ -162,7 +177,7 @@ fn test_blinking_system_normal_interval_odd_intervals() {
run_blinking_system(&mut world, 5);
// Should toggle twice (even number), so back to original state (not hidden)
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
// Check that timer was updated to remainder
let blinking = world.entity(entity).get::<Blinking>().unwrap();
@@ -178,7 +193,7 @@ fn test_blinking_system_zero_interval_with_ticks() {
run_blinking_system(&mut world, 1);
// Entity should be hidden immediately
assert_that(&has_hidden_component(&world, entity)).is_true();
assert_that(&is_entity_hidden(&world, entity)).is_true();
}
#[test]
@@ -190,7 +205,7 @@ fn test_blinking_system_zero_interval_no_ticks() {
run_blinking_system(&mut world, 0);
// Entity should not be hidden (no time passed)
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
}
#[test]
@@ -202,7 +217,7 @@ fn test_blinking_system_zero_interval_toggle_back() {
run_blinking_system(&mut world, 1);
// Entity should be unhidden
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
}
#[test]
@@ -214,7 +229,7 @@ fn test_blinking_system_frozen_entity_unhidden() {
run_blinking_system(&mut world, 10);
// Frozen entity should be unhidden and stay unhidden
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
assert_that(&has_frozen_component(&world, entity)).is_true();
}
@@ -227,7 +242,7 @@ fn test_blinking_system_frozen_entity_no_blinking() {
run_blinking_system(&mut world, 10);
// Frozen entity should not be hidden (blinking disabled)
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
assert_that(&has_frozen_component(&world, entity)).is_true();
}
@@ -255,7 +270,7 @@ fn test_blinking_system_entity_without_renderable_ignored() {
run_blinking_system(&mut world, 10);
// Entity should not be affected (not in query)
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
}
#[test]
@@ -274,7 +289,7 @@ fn test_blinking_system_entity_without_blinking_ignored() {
run_blinking_system(&mut world, 10);
// Entity should not be affected (not in query)
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
}
#[test]
@@ -286,7 +301,7 @@ fn test_blinking_system_large_interval() {
run_blinking_system(&mut world, 500);
// Entity should not be hidden yet
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
// Check that timer was updated
let blinking = world.entity(entity).get::<Blinking>().unwrap();
@@ -302,11 +317,11 @@ fn test_blinking_system_very_small_interval() {
run_blinking_system(&mut world, 1);
// Entity should be hidden
assert_that(&has_hidden_component(&world, entity)).is_true();
assert_that(&is_entity_hidden(&world, entity)).is_true();
// Run system with another 1 tick
run_blinking_system(&mut world, 1);
// Entity should be unhidden
assert_that(&has_hidden_component(&world, entity)).is_false();
assert_that(&is_entity_visible(&world, entity)).is_true();
}