Fix issue with the region subscription system not updating on view

distance changes (until the player crossed a chunk boundary and
triggered the normal update).

This introduces a `ViewDistance` struct that provides an abstraction
around limiting the rate the view distance can be cycled up and down.
This helps avoid unnecessary sending, deleting, and then resending of
synced things like entities (the client will still delete its terrain
locally and re-request it though).

The second part of this fix is storing the last view distance in the
`RegionSubscription` struct and then updating region subscriptions if
this doesn't match the current view distance in the `Presence`
component.
This commit is contained in:
Imbris 2022-08-21 00:48:51 -04:00
parent 638d91e58a
commit c36d6e873f
8 changed files with 139 additions and 31 deletions

View File

@ -1,12 +1,14 @@
use common_net::msg::PresenceKind;
use hashbrown::HashSet;
use serde::{Deserialize, Serialize};
use specs::{Component, DenseVecStorage, DerefFlaggedStorage, NullStorage, VecStorage};
use specs::{Component, DerefFlaggedStorage, NullStorage};
use std::time::{Duration, Instant};
use vek::*;
#[derive(Clone, Debug, Serialize, Deserialize)]
#[derive(Debug)]
pub struct Presence {
pub view_distance: u32,
// TODO: separate view distance for syncing entities?
pub view_distance: ViewDistance,
pub kind: PresenceKind,
pub lossy_terrain_compression: bool,
}
@ -14,7 +16,7 @@ pub struct Presence {
impl Presence {
pub fn new(view_distance: u32, kind: PresenceKind) -> Self {
Self {
view_distance,
view_distance: ViewDistance::new(view_distance, Instant::now()),
kind,
lossy_terrain_compression: false,
}
@ -22,8 +24,7 @@ impl Presence {
}
impl Component for Presence {
// Presence seems <= 64 bits, so it isn't worth using DenseVecStorage.
type Storage = DerefFlaggedStorage<Self, VecStorage<Self>>;
type Storage = DerefFlaggedStorage<Self, specs::DenseVecStorage<Self>>;
}
// Distance from fuzzy_chunk before snapping to current chunk
@ -34,11 +35,12 @@ pub const REGION_FUZZ: u32 = 16;
#[derive(Clone, Debug)]
pub struct RegionSubscription {
pub fuzzy_chunk: Vec2<i32>,
pub last_view_distance: u32,
pub regions: HashSet<Vec2<i32>>,
}
impl Component for RegionSubscription {
type Storage = DerefFlaggedStorage<Self, DenseVecStorage<Self>>;
type Storage = DerefFlaggedStorage<Self, specs::DenseVecStorage<Self>>;
}
#[derive(Copy, Clone, Debug, Default, Serialize, Deserialize)]
@ -47,3 +49,88 @@ pub struct RepositionOnChunkLoad;
impl Component for RepositionOnChunkLoad {
type Storage = NullStorage<Self>;
}
#[derive(PartialEq, Debug, Clone, Copy)]
enum Direction {
Up,
Down,
}
/// Distance from the [Presence] from which the world is loaded and information
/// is synced to clients.
///
/// We limit the frequency that changes in the view distance change direction
/// (e.g. shifting from increasing the value to decreasing it). This is useful
/// since we want to avoid rapid cycles of shrinking and expanding of the view
/// distance.
#[derive(Debug)]
pub struct ViewDistance {
direction: Direction,
last_direction_change_time: Instant,
target: Option<u32>,
current: u32,
}
impl ViewDistance {
/// Minimum time allowed between changes in direction of value adjustments.
const TIME_PER_DIR_CHANGE: Duration = Duration::from_millis(300);
pub fn new(start_value: u32, now: Instant) -> Self {
Self {
direction: Direction::Up,
last_direction_change_time: now - Self::TIME_PER_DIR_CHANGE,
target: None,
current: start_value,
}
}
/// Returns the current value.
pub fn current(&self) -> u32 { self.current }
/// Applies deferred change based on the whether the time to apply it has
/// been reached.
pub fn update(&mut self, now: Instant) {
if let Some(target_val) = self.target {
if now.saturating_duration_since(self.last_direction_change_time)
> Self::TIME_PER_DIR_CHANGE
{
self.last_direction_change_time = now;
self.current = target_val;
self.target = None;
}
}
}
/// Sets the target value.
///
/// If this hasn't been changed recently or it is in the same direction as
/// the previous change it will be applied immediately. Otherwise, it
/// will be deferred to a later time (limiting the frequency of changes
/// in the change direction).
pub fn set_target(&mut self, new_target: u32, now: Instant) {
use core::cmp::Ordering;
let new_direction = match new_target.cmp(&self.current) {
Ordering::Equal => return, // No change needed.
Ordering::Less => Direction::Down,
Ordering::Greater => Direction::Up,
};
// Change is in the same direction as before so we can just apply it.
if new_direction == self.direction {
self.current = new_target;
self.target = None;
// If it has already been a while since the last direction change we can
// directly apply the request and switch the direction.
} else if now.saturating_duration_since(self.last_direction_change_time)
> Self::TIME_PER_DIR_CHANGE
{
self.direction = new_direction;
self.last_direction_change_time = now;
self.current = new_target;
self.target = None;
// Otherwise, we need to defer the request.
} else {
self.target = Some(new_target);
}
}
}

View File

@ -519,7 +519,7 @@ impl StateExt for State {
// Make sure physics components are updated
self.write_component_ignore_entity_dead(entity, comp::ForceUpdate::forced());
const INITIAL_VD: u32 = 5; //will be changed after login
const INITIAL_VD: u32 = 5; // will be changed after login
self.write_component_ignore_entity_dead(
entity,
Presence::new(INITIAL_VD, PresenceKind::Character(character_id)),

View File

@ -384,7 +384,8 @@ impl<'a> System<'a> for Sys {
let is_near = |o_pos: Vec3<f32>| {
pos.zip_with(presence, |pos, presence| {
pos.0.xy().distance_squared(o_pos.xy())
< (presence.view_distance as f32 * TerrainChunkSize::RECT_SIZE.x as f32)
< (presence.view_distance.current() as f32
* TerrainChunkSize::RECT_SIZE.x as f32)
.powi(2)
})
};

View File

@ -17,6 +17,7 @@ use common_ecs::{Job, Origin, Phase, System};
use common_net::msg::{ClientGeneral, PresenceKind, ServerGeneral};
use common_state::{BlockChange, BuildAreas};
use specs::{Entities, Join, Read, ReadExpect, ReadStorage, Write, WriteStorage};
use std::time::Instant;
use tracing::{debug, trace, warn};
use vek::*;
@ -49,6 +50,7 @@ impl Sys {
_terrain_persistence: &mut TerrainPersistenceData<'_>,
maybe_player: &Option<&Player>,
maybe_admin: &Option<&Admin>,
time_for_vd_changes: Instant,
msg: ClientGeneral,
) -> Result<(), crate::error::Error> {
let presence = match maybe_presence {
@ -67,20 +69,15 @@ impl Sys {
*maybe_presence = None;
},
ClientGeneral::SetViewDistance(view_distance) => {
presence.view_distance = settings
let clamped_view_distance = settings
.max_view_distance
.map(|max| view_distance.min(max))
.unwrap_or(view_distance);
presence.view_distance.set_target(clamped_view_distance, time_for_vd_changes);
//correct client if its VD is to high
if settings
.max_view_distance
.map(|max| view_distance > max)
.unwrap_or(false)
{
client.send(ServerGeneral::SetViewDistance(
settings.max_view_distance.unwrap_or(0),
))?;
// Correct client if its requested VD is too high.
if view_distance != clamped_view_distance {
client.send(ServerGeneral::SetViewDistance(clamped_view_distance))?;
}
},
ClientGeneral::ControllerInputs(inputs) => {
@ -378,6 +375,8 @@ impl<'a> System<'a> for Sys {
) {
let mut server_emitter = server_event_bus.emitter();
let time_for_vd_changes = Instant::now();
for (entity, client, mut maybe_presence, player, maybe_admin) in (
&entities,
&mut clients,
@ -387,12 +386,15 @@ impl<'a> System<'a> for Sys {
)
.join()
{
// If an `ExitInGame` message is received this is set to `None` allowing further
// ingame messages to be ignored.
let mut clearable_maybe_presence = maybe_presence.as_deref_mut();
let _ = super::try_recv_all(client, 2, |client, msg| {
Self::handle_client_in_game_msg(
&mut server_emitter,
entity,
client,
&mut maybe_presence.as_deref_mut(),
&mut clearable_maybe_presence,
&terrain,
&can_build,
&is_rider,
@ -410,9 +412,16 @@ impl<'a> System<'a> for Sys {
&mut terrain_persistence,
&player,
&maybe_admin,
time_for_vd_changes,
msg,
)
});
// Ensure deferred view distance changes are applied (if the
// requsite time has elapsed).
if let Some(mut presence) = maybe_presence {
presence.view_distance.update(time_for_vd_changes);
}
}
}
}

View File

@ -76,7 +76,7 @@ impl<'a> System<'a> for Sys {
pos.0.xy().map(|e| e as f64).distance_squared(
key.map(|e| e as f64 + 0.5)
* TerrainChunkSize::RECT_SIZE.map(|e| e as f64),
) < ((presence.view_distance as f64 - 1.0
) < ((presence.view_distance.current() as f64 - 1.0
+ 2.5 * 2.0_f64.sqrt())
* TerrainChunkSize::RECT_SIZE.x as f64)
.powi(2)

View File

@ -60,7 +60,8 @@ impl<'a> System<'a> for Sys {
// To update subscriptions
// 1. Iterate through clients
// 2. Calculate current chunk position
// 3. If chunk is the same return, otherwise continue (use fuzziness)
// 3. If chunk is different (use fuzziness) or the client view distance
// has changed continue, otherwise return
// 4. Iterate through subscribed regions
// 5. Check if region is still in range (use fuzziness)
// 6. If not in range
@ -78,13 +79,15 @@ impl<'a> System<'a> for Sys {
)
.join()
{
let vd = presence.view_distance;
let vd = presence.view_distance.current();
// Calculate current chunk
let chunk = (Vec2::<f32>::from(pos.0))
.map2(TerrainChunkSize::RECT_SIZE, |e, sz| e as i32 / sz as i32);
// Only update regions when moving to a new chunk
// uses a fuzzy border to prevent rapid triggering when moving along chunk
// boundaries
// Only update regions when moving to a new chunk or if view distance has
// changed.
//
// Uses a fuzzy border to prevent rapid triggering when moving along chunk
// boundaries.
if chunk != subscription.fuzzy_chunk
&& (subscription
.fuzzy_chunk
@ -96,7 +99,10 @@ impl<'a> System<'a> for Sys {
e.abs() > (sz / 2 + presence::CHUNK_FUZZ) as f32
})
.reduce_or()
|| subscription.last_view_distance != vd
{
// Update the view distance
subscription.last_view_distance = vd;
// Update current chunk
subscription.fuzzy_chunk = Vec2::<f32>::from(pos.0)
.map2(TerrainChunkSize::RECT_SIZE, |e, sz| e as i32 / sz as i32);
@ -216,7 +222,7 @@ pub fn initialize_region_subscription(world: &World, entity: specs::Entity) {
let chunk_size = TerrainChunkSize::RECT_SIZE.reduce_max() as f32;
let regions = regions_in_vd(
client_pos.0,
(presence.view_distance as f32 * chunk_size) as f32
(presence.view_distance.current() as f32 * chunk_size) as f32
+ (presence::CHUNK_FUZZ as f32 + chunk_size) * 2.0f32.sqrt(),
);
@ -261,6 +267,7 @@ pub fn initialize_region_subscription(world: &World, entity: specs::Entity) {
if let Err(e) = world.write_storage().insert(entity, RegionSubscription {
fuzzy_chunk,
last_view_distance: presence.view_distance.current(),
regions,
}) {
error!(?e, "Failed to insert region subscription component");

View File

@ -269,7 +269,7 @@ impl<'a> System<'a> for Sys {
.map(|e: i32| (e.unsigned_abs()).saturating_sub(2))
.magnitude_squared();
if adjusted_dist_sqr <= presence.view_distance.pow(2) {
if adjusted_dist_sqr <= presence.view_distance.current().pow(2) {
chunk_send_emitter.emit(ChunkSendEntry {
entity,
chunk_key: key,
@ -293,7 +293,7 @@ impl<'a> System<'a> for Sys {
// For each player with a position, calculate the distance.
for (presence, pos) in (&presences, &positions).join() {
if chunk_in_vd(pos.0, chunk_key, &terrain, presence.view_distance) {
if chunk_in_vd(pos.0, chunk_key, &terrain, presence.view_distance.current()) {
should_drop = false;
break;
}

View File

@ -33,8 +33,12 @@ impl<'a> System<'a> for Sys {
// Sync changed chunks
for chunk_key in &terrain_changes.modified_chunks {
for (entity, presence, pos) in (&entities, &presences, &positions).join() {
if super::terrain::chunk_in_vd(pos.0, *chunk_key, &terrain, presence.view_distance)
{
if super::terrain::chunk_in_vd(
pos.0,
*chunk_key,
&terrain,
presence.view_distance.current(),
) {
chunk_send_emitter.emit(ChunkSendEntry {
entity,
chunk_key: *chunk_key,