addressed review comments

This commit is contained in:
crabman 2024-02-27 08:37:40 +00:00
parent 036e79284e
commit a14c2f054c
No known key found for this signature in database
6 changed files with 60 additions and 20 deletions

View File

@ -84,6 +84,7 @@ command-repaired-items = Repaired all equipped items
command-message-group-missing = You are using group chat but do not belong to a group. Use /world or command-message-group-missing = You are using group chat but do not belong to a group. Use /world or
/region to change chat. /region to change chat.
command-tell-request = { $sender } wants to talk to you. command-tell-request = { $sender } wants to talk to you.
command-transform-invalid-presence = Cannot transform in the current presence
# Unreachable/untestable but added for consistency # Unreachable/untestable but added for consistency

View File

@ -3,6 +3,7 @@ use std::time::Duration;
use common_assets::AssetExt; use common_assets::AssetExt;
use rand::thread_rng; use rand::thread_rng;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use tracing::error;
use vek::Vec3; use vek::Vec3;
use crate::{ use crate::{
@ -65,6 +66,7 @@ impl CharacterBehavior for Data {
// Buildup finished, start transformation // Buildup finished, start transformation
} else { } else {
let Ok(entity_config) = EntityConfig::load(&self.static_data.target) else { let Ok(entity_config) = EntityConfig::load(&self.static_data.target) else {
error!(?self.static_data.target, "Failed to load entity configuration");
end_ability(data, &mut update); end_ability(data, &mut update);
return update; return update;
}; };

View File

@ -672,9 +672,10 @@ fn handle_into_npc(
TransformEntityError::UnexpectedNpcTeleporter => { TransformEntityError::UnexpectedNpcTeleporter => {
Content::localized("command-unimplemented-teleporter-spawn") Content::localized("command-unimplemented-teleporter-spawn")
}, },
})?; TransformEntityError::LoadingCharacter => {
Content::localized("command-transform-invalid-presence")
Ok(()) },
})
} }
fn handle_make_npc( fn handle_make_npc(

View File

@ -22,7 +22,7 @@ use common::{
item::flatten_counted_items, item::flatten_counted_items,
loot_owner::LootOwnerKind, loot_owner::LootOwnerKind,
Alignment, Auras, Body, CharacterState, Energy, Group, Health, Inventory, Object, Player, Alignment, Auras, Body, CharacterState, Energy, Group, Health, Inventory, Object, Player,
Poise, Pos, Presence, PresenceKind, SkillSet, Stats, Poise, Pos, Presence, PresenceKind, SkillSet, Stats, BASE_ABILITY_LIMIT,
}, },
consts::TELEPORTER_RADIUS, consts::TELEPORTER_RADIUS,
event::{ event::{
@ -2106,13 +2106,17 @@ pub fn handle_transform(server: &mut Server, TransformEvent(uid, info): Transfor
return; return;
}; };
let _ = transform_entity(server, entity, info); if let Err(error) = transform_entity(server, entity, info) {
error!(?error, ?uid, "Failed transform entity");
}
} }
#[derive(Debug)]
pub enum TransformEntityError { pub enum TransformEntityError {
EntityDead, EntityDead,
UnexpectedNpcWaypoint, UnexpectedNpcWaypoint,
UnexpectedNpcTeleporter, UnexpectedNpcTeleporter,
LoadingCharacter,
} }
pub fn transform_entity( pub fn transform_entity(
@ -2158,20 +2162,38 @@ pub fn transform_entity(
} }
// Disable persistence // Disable persistence
'persist: {
match server
.state
.ecs()
.read_storage::<Presence>()
.get(entity)
.map(|presence| presence.kind)
{ {
// Transforming while the character is being loaded or is spectating is invalid!
Some(PresenceKind::Spectator | PresenceKind::LoadingCharacter(_)) => {
return Err(TransformEntityError::LoadingCharacter);
},
Some(PresenceKind::Possessor | PresenceKind::Character(_)) => {},
None => break 'persist,
}
// Run persistence once before disabling it // Run persistence once before disabling it
//
// We must NOT early return between persist_entity() being called and
// persistence being set to Possessor
super::player::persist_entity(server.state_mut(), entity); super::player::persist_entity(server.state_mut(), entity);
// TODO: let Imbris work out some edge-cases: // We re-fetch presence here as mutable, because checking for a valid
// - error on PresenseKind::LoadingCharacter // [`PresenceKind`] must be done BEFORE persist_entity but persist_entity needs
// - handle active inventory actions // exclusive mutable access to the server's state
let ecs = server.state.ecs(); let mut presences = server.state.ecs().write_storage::<Presence>();
let mut presences = ecs.write_storage::<Presence>(); let Some(presence) = presences.get_mut(entity) else {
let presence = presences.get_mut(entity); // Checked above
unreachable!("We already know this entity has a Presence");
};
if let Some(presence) = presence if let PresenceKind::Character(id) = presence.kind {
&& let PresenceKind::Character(id) = presence.kind
{
server.state.ecs().write_resource::<IdMaps>().remove_entity( server.state.ecs().write_resource::<IdMaps>().remove_entity(
Some(entity), Some(entity),
None, None,
@ -2194,6 +2216,16 @@ pub fn transform_entity(
set_or_remove_component(server, entity, Some(body.density()))?; set_or_remove_component(server, entity, Some(body.density()))?;
set_or_remove_component(server, entity, Some(body.collider()))?; set_or_remove_component(server, entity, Some(body.collider()))?;
set_or_remove_component(server, entity, Some(scale))?; set_or_remove_component(server, entity, Some(scale))?;
// Reset active abilities
set_or_remove_component(
server,
entity,
Some(if body.is_humanoid() {
comp::ActiveAbilities::default_limited(BASE_ABILITY_LIMIT)
} else {
comp::ActiveAbilities::default()
}),
)?;
// Don't add Agent or ItemDrops to players // Don't add Agent or ItemDrops to players
if !is_player { if !is_player {

View File

@ -37,6 +37,7 @@ mod mounting;
mod player; mod player;
mod trade; mod trade;
/// Shared utilities used by other code **in this crate**
pub(crate) mod shared { pub(crate) mod shared {
pub(crate) use super::{ pub(crate) use super::{
entity_manipulation::{transform_entity, TransformEntityError}, entity_manipulation::{transform_entity, TransformEntityError},

View File

@ -243,11 +243,14 @@ pub fn handle_client_disconnect(
Event::ClientDisconnected { entity } Event::ClientDisconnected { entity }
} }
// When a player logs out, their data is queued for persistence in the next tick /// When a player logs out, their data is queued for persistence in the next
// of the persistence batch update. The player will be /// tick of the persistence batch update. The player will be
// temporarily unable to log in during this period to avoid /// temporarily unable to log in during this period to avoid
// the race condition of their login fetching their old data /// the race condition of their login fetching their old data
// and overwriting the data saved here. /// and overwriting the data saved here.
///
/// This function is also used by the Transform event and MUST NOT assume that
/// the persisting entity is deleted afterwards.
pub(super) fn persist_entity(state: &mut State, entity: EcsEntity) -> EcsEntity { pub(super) fn persist_entity(state: &mut State, entity: EcsEntity) -> EcsEntity {
if let ( if let (
Some(presence), Some(presence),