Skip to content

Aabb #182

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.

Sign up for GitHub

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

Closed
DimpyRed wants to merge 20 commits into from
Closed

Aabb #182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
56c28d0
Merge pull request #1 from Ralith/master
DimpyRed Jan 27, 2021
4a43b68
Made a few refactors to make the linter happy
Jun 15, 2021
1037c5a
added AABB masking
Jun 15, 2021
49c1930
preliminary tested AABB
Jul 22, 2021
4163172
Tweaks, bug fixes, and better testing of AABBs
Jan 20, 2022
2763c2e
added velocity, gravity, and soft collisions
Mar 26, 2022
715357e
added velocity, gravity, and soft collisions
Mar 26, 2022
3aea322
Merge branch 'AABB' of https://github.com/DimpyRed/hypermine into AABB
May 17, 2022
da6ba5a
finished the merge
May 17, 2022
167d222
Cleanup of force.rs
May 24, 2022
b76e66d
gravity/velocity parameters are configurable
May 24, 2022
36153bc
more fixes to bounding box accuraccy
Jun 25, 2022
3bae7d5
General comment cleanup
Jul 28, 2022
70caa9f
Merge branch 'master' into AABB
Jul 28, 2022
8a7aa94
post-merge refactoring
Jul 29, 2022
d315a4d
Implemented Debug display for Bounding Boxes
Aug 2, 2022
d68fcfa
trivial suggested changes
Aug 7, 2022
8ddee5a
code review refactoring
Aug 10, 2022
ec9abf5
Recalibrate reasonable voxel count test
Aug 15, 2022
4405b9c
more trival corrections
Aug 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 167 additions & 7 deletions client/src/sim.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,21 @@ use tracing::{debug, error, trace};

use crate::{net, prediction::PredictedMotion, Net};
use common::{
collision::BoundingBox,
force::{ForceParams, GravityMethod},
graph::{Graph, NodeId},
math,
node::{DualGraph, Node},
node::{Chunk, DualGraph, Node},
proto::{self, Character, Command, Component, Position},
sanitize_motion_input,
world::Material,
worldgen::NodeState,
Chunks, EntityId, GraphEntities, Step,
};

const CHARACTER_RADIUS: f64 = 0.10_f64;
const CHARACTER_SLOWDOWN_FACTOR: f32 = 6_f32;
Copy link
Collaborator

@patowen patowen Aug 6, 2022

Choose a reason for hiding this comment

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

Should these two constants be part of sim_config?


/// Game state
pub struct Sim {
net: Net,
Expand All @@ -26,6 +32,7 @@ pub struct Sim {
pub world: hecs::World,
pub params: Option<Parameters>,
pub local_character: Option<Entity>,
character_velocity: na::Vector4<f32>, // velocity of the character controller that carries over from frame to frame.
Copy link
Collaborator

@patowen patowen Aug 6, 2022

Choose a reason for hiding this comment

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

Currently, this velocity is computed in a coordinate system relative to the current node the player is associated with. I have personally found it easier to reason about velocity when it was provided in the player's own coordinate system. This way, the velocity doesn't need to be renormalized every frame. The main thing to worry about is that if the player's coordinate system is rotated, the velocity vector would need to be rotated the other way to stay consistent.

Having the velocity be relative to the current node works as well, but it requires a lot of care to keep it consistent with the current coordinate system of the player.

Copy link
Contributor Author

@DimpyRed DimpyRed Aug 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I see no reason that the player's coordinate system would be rotated such that a rotation of velocity would not also be warranted.
This sounds like the way to go.

Copy link
Collaborator

@patowen patowen Aug 6, 2022

Choose a reason for hiding this comment

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

To remain consistent with the current use of hecs, velocity should be added to the local_character entity somehow. I'm not familiar with best practices here, but one possibility is to add velocity to the Character struct in proto.rs, as I believe the purpose of this struct is to store information related to players (and possibly NPCs?). Another possibility is to have a separate Velocity struct, but I don't really see the benefit right now for this kind of voxel game.

orientation: na::UnitQuaternion<f32>,
step: Option<Step>,

Expand Down Expand Up @@ -54,6 +61,7 @@ impl Sim {
world: hecs::World::new(),
params: None,
local_character: None,
character_velocity: na::Vector4::zeros(),
orientation: na::one(),
step: None,

Expand Down Expand Up @@ -86,6 +94,8 @@ impl Sim {
self.handle_net(msg);
}

self.handle_forces(dt);

if let Some(step_interval) = self.params.as_ref().map(|x| x.step_interval) {
self.since_input_sent += dt;
if let Some(overflow) = self.since_input_sent.checked_sub(step_interval) {
Expand Down Expand Up @@ -135,6 +145,9 @@ impl Sim {
chunk_size: msg.chunk_size,
meters_to_absolute: msg.meters_to_absolute,
movement_speed: msg.movement_speed,
gravity_intensity: msg.gravity_intensity,
drag_factor: msg.drag_factor,
gravity_method: msg.gravity_method,
});
// Populate the root node
populate_fresh_nodes(&mut self.graph);
Expand All @@ -146,8 +159,8 @@ impl Sim {
return;
}
self.step = Some(msg.step);
for &(id, new_pos) in &msg.positions {
self.update_position(msg.latest_input, id, new_pos);
for &(id, new_pos, node_transform) in &msg.positions {
self.update_position(msg.latest_input, id, new_pos, node_transform);
Copy link
Collaborator

@patowen patowen Aug 7, 2022

Choose a reason for hiding this comment

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

I'm not sure why, but with some println debugging, I've found situations in which node_transform was the identity matrix, but new_pos was different from the old position. I believe this is the main cause of the velocity redirection you noticed recently.

Rather than figuring out the root cause of this discrepancy, I believe the simplest fix would be to do this while fixing the jitter. For prototyping purposes, to simplify development, I would recommend having the client simply send the entire Position structure to the server and never have the server reposition the client. Once we have a better understanding of netcode, we can try to do things the right way.

Copy link
Contributor Author

@DimpyRed DimpyRed Aug 7, 2022

Choose a reason for hiding this comment

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

I don't think I made it very clear, but node_transform is only not the identity matrix when going from one node to another node.
When the player is moving around in the middle of the node, the exact scenario you are describing should be the norm.

I agree that the Client broadcasting its Position is the proper thing to do right now.

Copy link
Collaborator

@patowen patowen Aug 7, 2022

Choose a reason for hiding this comment

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

Sorry, I meant that new_pos.old was different from the old pos.node. I'm aware that node_transform being the identity is correct when staying within a single node.

}
for &(id, orientation) in &msg.character_orientations {
match self.entity_ids.get(&id) {
Expand All @@ -166,14 +179,29 @@ impl Sim {
}
}

fn update_position(&mut self, latest_input: u16, id: EntityId, new_pos: Position) {
if self.params.as_ref().map_or(false, |x| x.character_id == id) {
fn update_position(
&mut self,
latest_input: u16,
id: EntityId,
new_pos: Position,
node_transform: na::Matrix4<f32>,
) {
let id_is_character = self.params.as_ref().map_or(false, |x| x.character_id == id);

if id_is_character {
self.prediction.reconcile(latest_input, new_pos);
}

match self.entity_ids.get(&id) {
None => debug!(%id, "position update for unknown entity"),
Some(&entity) => match self.world.get_mut::<Position>(entity) {
Ok(mut pos) => {
if id_is_character {
let p0 = node_transform * pos.local * math::origin();
let p1 = new_pos.local * math::origin();
self.character_velocity =
math::translate(&p0, &p1) * node_transform * self.character_velocity;
}
if pos.node != new_pos.node {
self.graph_entities.remove(pos.node, entity);
self.graph_entities.insert(new_pos.node, entity);
Expand Down Expand Up @@ -241,8 +269,24 @@ impl Sim {
}

fn send_input(&mut self) {
let (direction, speed) = sanitize_motion_input(self.orientation * self.average_velocity);
let params = self.params.as_ref().unwrap();
let local_velocity = self
.world
.get_mut::<Position>(self.local_character.unwrap())
.unwrap()
.local
.try_inverse()
.unwrap()
* self.character_velocity;
let (direction, speed) = sanitize_motion_input(
self.orientation * self.average_velocity / CHARACTER_SLOWDOWN_FACTOR // lower player abilities
Copy link
Collaborator

@patowen patowen Aug 7, 2022

Choose a reason for hiding this comment

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

Currently, self.average_velocity and local_velocity are treated entirely independently. It may be better for the arrow keys to influence the persistent velocity, as that would make the code simpler to reason about. Since the controls are currently a bit of a hybrid between noclip and gravity, it's not clear to me how this would work. One possibility would be for movement to be controlled by the player if any movement key is held down but for it to be controlled by gravity otherwise.

Copy link
Contributor Author

@DimpyRed DimpyRed Aug 7, 2022

Choose a reason for hiding this comment

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

I suppose that could work if I made ground drag only apply in the vertical direction.

There are two downsides I see to implementing it right now:

  1. We lose the legacy noclip-style of movement that's useful for surveying the scenery
  2. The weird holonomy of moving in a different direction than the actual velocity is puts bug-revealing stresses on the system.

average_velocity should definitely be renamed though.

Copy link
Collaborator

@patowen patowen Aug 22, 2022

Choose a reason for hiding this comment

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

Point (1) can be handled with a noclip toggle, but point (2) makes sense. Although, revealing bugs may be a good thing, as that gives us a chance to fix them.

+ na::Vector3::new(
local_velocity[0],
local_velocity[1],
local_velocity[2],
) / params.movement_speed,
);

let generation = self.prediction.push(
&direction,
speed
Expand All @@ -263,7 +307,10 @@ impl Sim {
result.local *= self.orientation.to_homogeneous();
if let Some(ref params) = self.params {
// Apply input that hasn't been sent yet
let (direction, speed) = sanitize_motion_input(self.average_velocity);
let (direction, speed) = sanitize_motion_input(
// TODO: Incorparate the persistent velocity into the view calculations.
Copy link
Collaborator

@patowen patowen Aug 7, 2022

Choose a reason for hiding this comment

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

The simplest way to do this may be to get self.average_velocity to be set based on the persistent velocity. There's already a self.instantaneous_velocity, which I believe the persistent velocity could be substituted in for.

self.average_velocity / CHARACTER_SLOWDOWN_FACTOR,
);
// We multiply by the entire timestep rather than the time so far because
// self.average_velocity is always over the entire timestep, filling in zeroes for the
// future.
Expand Down Expand Up @@ -292,6 +339,116 @@ impl Sim {
.despawn(entity)
.expect("destroyed nonexistent entity");
}

fn handle_forces(&mut self, time: Duration) {
let params = self.params.as_ref().unwrap();

let force_system = ForceParams {
gravity_intensity: params.gravity_intensity,
air_drag_factor: params.drag_factor,
gravity_type: params.gravity_method,
float_speed: 0.05_f64,
};

// eventually this should be expanded to work on every entity with a physics property, but for now it is just the player
match self.local_character {
Some(entity) => match self.world.get_mut::<Position>(entity) {
Ok(character_position) => {
// starting with simpler method for testing purposese
let is_colliding = self.check_collision(*character_position);

let down_info = &self
.graph
.get(character_position.node)
.as_ref()
.unwrap()
.state;

let character_v4f64 = na::convert::<_, na::Vector4<f64>>(
character_position.local * math::origin(),
);

let down = -math::lorentz_normalize(&math::orthogonalize(
down_info.surface().normal(),
&character_v4f64,
));

let down_local = self.orientation.conjugate()
* (character_position.local.try_inverse().unwrap()
* na::convert::<_, na::Vector4<f32>>(-down))
.xyz();
self.orientation *= na::UnitQuaternion::new(
na::Vector3::z() * down_local.x * -3.0 * time.as_secs_f32(),
);

let height = down_info.surface().distance_to(&character_v4f64);

let new_velocity = force_system.apply_forces(
&na::convert::<_, na::Vector4<f64>>(self.character_velocity),
&down,
height,
is_colliding,
time.as_secs_f64(),
);

self.character_velocity = math::normalize_vector(
character_position.local,
na::convert::<_, na::Vector4<f32>>(new_velocity),
);

for n in self.character_velocity.iter() {
assert!(!n.is_nan(), "Error, character velocity is not a number");
}
}
Err(_e) => (),
},
None => (),
}
}

/// checks to see if there are any non-Void voxels near Position.
// acts as a helper function for handle_forces
// currently uses a hyperbolic equivilent of an AABB.
fn check_collision(&self, pos: Position) -> bool {
let params = self.params.as_ref().unwrap();

let bb = BoundingBox::create_aabb(
pos.node,
na::convert::<na::Vector4<f32>, na::Vector4<f64>>(
pos.local * na::Vector4::new(0_f32, 0_f32, 0_f32, 1_f32),
),
CHARACTER_RADIUS,
&self.graph,
params.chunk_size,
);

for cbb in bb.bounding_boxes {
if match self
.graph
.get(cbb.node)
.as_ref()
.expect("nodes must be populated before collisons can occur")
.chunks[cbb.chunk]
{
Chunk::Generating => true,
Chunk::Fresh => true,
Chunk::Populated {
surface: _,
ref voxels,
} => {
for voxel_address in cbb.every_voxel() {
if !matches!(voxels.get(voxel_address as usize), Material::Void) {
return true;
}
}
false
}
} {
return true;
}
}
false
}
}

/// Simulation details received on connect
Expand All @@ -302,6 +459,9 @@ pub struct Parameters {
/// Absolute units
pub movement_speed: f32,
pub character_id: EntityId,
pub gravity_intensity: f64,
pub drag_factor: f64,
pub gravity_method: GravityMethod,
}

fn populate_fresh_nodes(graph: &mut DualGraph) {
Expand Down
Loading