Skip to content

Put ClientDiagnosticsPlugin under diagnostics feature #295

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

Merged
merged 6 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
run: cargo clippy --benches --tests -- -D warnings

- name: Rustdoc
run: cargo rustdoc -- -D warnings
run: cargo rustdoc --all-features -- -D warnings

doctest:
name: Doctest
Expand All @@ -66,7 +66,7 @@ jobs:
uses: Swatinem/rust-cache@v2

- name: Test doc
run: cargo test --doc
run: cargo test --all-features --doc

test:
name: Test
Expand All @@ -88,7 +88,7 @@ jobs:
run: cargo install cargo-tarpaulin

- name: Test
run: cargo tarpaulin --engine llvm --out lcov --exclude-files benches/* --exclude-files bevy_replicon_renet/*
run: cargo tarpaulin --all-features --engine llvm --out lcov --exclude-files benches/* --exclude-files bevy_replicon_renet/*

- name: Upload code coverage results
if: github.actor != 'dependabot[bot]'
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- `ServerEventsPlugin` and `ClientEventsPlugin` can be disabled on client-only and server-only apps respectively.
- Put `ClientDiagnosticsPlugin` under `diagnostics` feature and make it part of the `RepliconPlugins` group.
- Do not divide values per seconds by the number of messages for `ClientDiagnosticsPlugin`.
- Move `server::events::event_data` module to `core::event_registry::server_event`.
- Move `client::events::event_data` module to `core::event_registry::client_event`.
Expand Down
20 changes: 17 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ categories = ["game-development", "network-programming"]
license = "MIT OR Apache-2.0"
include = ["/benches", "/src", "/tests", "/LICENSE*"]

[package.metadata.docs.rs]
rustdoc-args = ["-Zunstable-options", "--cfg", "docsrs"]
all-features = true

[dependencies]
bevy = { version = "0.14.0-rc.3", default-features = false, features = [
"bevy_scene",
Expand All @@ -40,14 +44,24 @@ criterion = { version = "0.5", default-features = false, features = [
"cargo_bench_support",
] }

[lints.clippy]
type_complexity = "allow"
too_many_arguments = "allow"
[features]
default = []

# Plugin for integration with Bevy diagnostics.
diagnostics = []

[[bench]]
name = "replication"
harness = false

[[test]]
name = "stats"
required-features = ["diagnostics"]

[lints.clippy]
type_complexity = "allow"
too_many_arguments = "allow"

# Removed until `bevy_renet` supports 0.14.0-rc.3.
# [workspace]
# members = ["bevy_replicon_renet"]
25 changes: 24 additions & 1 deletion src/client.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod confirm_history;
#[cfg(feature = "diagnostics")]
pub mod diagnostics;
pub mod events;
pub mod replicon_client;
Expand All @@ -21,7 +22,6 @@ use crate::core::{
Replicated,
};
use confirm_history::ConfirmHistory;
use diagnostics::ClientStats;
use replicon_client::RepliconClient;
use server_entity_map::ServerEntityMap;

Expand Down Expand Up @@ -640,3 +640,26 @@ pub(super) struct BufferedUpdate {
/// Update data.
message: Bytes,
}

/// Replication stats during message processing.
///
/// Statistic will be collected only if the resource is present.
/// The resource is not added by default.
Comment on lines +646 to +647
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// Statistic will be collected only if the resource is present.
/// The resource is not added by default.
/// Statistic will be collected only if the resource is present.
/// The resource is not added by default. See [`ClientDiagnosticsPlugin`].

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 thought about it, but it's under a feature :(

If I link to it, cargo doc --open will emit a warning about missing link. I will need to enable the feature explicitly in CLI to generate docs without warnings.

Maybe you know a way to conditionally include docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. I will just always run --all-features for cargo doc to avoid warnings. Discoverability is important.

Copy link
Collaborator

@UkoeHB UkoeHB Jun 25, 2024

Choose a reason for hiding this comment

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

Maybe you know a way to conditionally include docs?

You should be able to get all docs with this (and also get the nice feature-requirement message in docs):

# Cargo.toml
[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]
#[cfg_attr(docsrs, doc(cfg(feature = "diagnostics")))]
pub mod diagnostics;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done!
I used doc_auto_cfg, like Bevy does:
bevyengine/bevy#12366
bevyengine/bevy#12642

///
/// See also [`ClientDiagnosticsPlugin`](diagnostics::ClientDiagnosticsPlugin)
/// for automatic integration with Bevy diagnostics.
#[derive(Default, Resource, Debug)]
pub struct ClientStats {
/// Incremented per entity that changes.
pub entities_changed: u32,
/// Incremented for every component that changes.
pub components_changed: u32,
/// Incremented per client mapping added.
pub mappings: u32,
/// Incremented per entity despawn.
pub despawns: u32,
/// Replication messages received.
pub messages: u32,
/// Replication bytes received in message payloads (without internal messaging plugin data).
pub bytes: u64,
}
93 changes: 38 additions & 55 deletions src/client/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,67 +6,50 @@ use bevy::{
};
use std::time::Duration;

/// Replication stats during message processing.
///
/// Flushed to Diagnostics system periodically.
#[derive(Default, Resource, Debug)]
pub struct ClientStats {
/// Incremented per entity that changes.
pub entities_changed: u32,
/// Incremented for every component that changes.
pub components_changed: u32,
/// Incremented per client mapping added.
pub mappings: u32,
/// Incremented per entity despawn.
pub despawns: u32,
/// Replication messages received.
pub messages: u32,
/// Replication bytes received in message payloads (without internal messaging plugin data).
pub bytes: u64,
}
use super::ClientStats;

/// Plugin to write Diagnostics every second.
/// Plugin to write [`Diagnostics`] based on [`ClientStats`] every second.
///
/// Not added by default.
/// Adds [`ClientStats`] resource and automatically resets it to get diagnostics per second.
pub struct ClientDiagnosticsPlugin;

impl Plugin for ClientDiagnosticsPlugin {
fn build(&self, app: &mut App) {
app.add_systems(
Update,
Self::add_measurements.run_if(on_timer(Duration::from_secs(1))),
)
.init_resource::<ClientStats>()
.register_diagnostic(
Diagnostic::new(Self::ENTITY_CHANGES)
.with_suffix("entities changed per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::COMPONENT_CHANGES)
.with_suffix("components changed per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::MAPPINGS)
.with_suffix("mappings added per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::DESPAWNS)
.with_suffix("despawns per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::MESSAGES)
.with_suffix("messages per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::BYTES)
.with_suffix("bytes per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
);
app.init_resource::<ClientStats>()
.add_systems(
Update,
Self::add_measurements.run_if(on_timer(Duration::from_secs(1))),
)
.register_diagnostic(
Diagnostic::new(Self::ENTITY_CHANGES)
.with_suffix("entities changed per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::COMPONENT_CHANGES)
.with_suffix("components changed per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::MAPPINGS)
.with_suffix("mappings added per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::DESPAWNS)
.with_suffix("despawns per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::MESSAGES)
.with_suffix("messages per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
)
.register_diagnostic(
Diagnostic::new(Self::BYTES)
.with_suffix("bytes per second")
.with_max_history_length(Self::DIAGNOSTIC_HISTORY_LEN),
);
}
}

Expand Down
19 changes: 15 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ To reduce packet size there are the following limits per replication update:
- Up to [`u16::MAX`] entities that have removed components with up to [`u16::MAX`] bytes of component data.
- Up to [`u16::MAX`] entities that were despawned.
*/
#![cfg_attr(docsrs, feature(doc_auto_cfg))]

pub mod client;
pub mod core;
Expand All @@ -453,10 +454,9 @@ pub mod prelude {

pub use super::{
client::{
diagnostics::{ClientDiagnosticsPlugin, ClientStats},
events::ClientEventsPlugin,
replicon_client::{RepliconClient, RepliconClientStatus},
ClientPlugin, ClientSet,
ClientPlugin, ClientSet, ClientStats,
},
core::{
channels::{ChannelKind, RepliconChannel, RepliconChannels},
Expand All @@ -481,6 +481,9 @@ pub mod prelude {
},
RepliconPlugins,
};

#[cfg(feature = "diagnostics")]
pub use super::client::diagnostics::ClientDiagnosticsPlugin;
}

pub use bincode;
Expand All @@ -493,12 +496,20 @@ pub struct RepliconPlugins;

impl PluginGroup for RepliconPlugins {
fn build(self) -> PluginGroupBuilder {
PluginGroupBuilder::start::<Self>()
let mut group = PluginGroupBuilder::start::<Self>();
group = group
.add(RepliconCorePlugin)
.add(ParentSyncPlugin)
.add(ClientPlugin)
.add(ServerPlugin::default())
.add(ClientEventsPlugin)
.add(ServerEventsPlugin)
.add(ServerEventsPlugin);

#[cfg(feature = "diagnostics")]
{
group = group.add(ClientDiagnosticsPlugin);
}

group
}
}
66 changes: 0 additions & 66 deletions tests/other.rs → tests/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use bevy_replicon::{
core::channels::ReplicationChannel, prelude::*, server::server_tick::ServerTick,
test_app::ServerTestAppExt,
};
use serde::{Deserialize, Serialize};

#[test]
fn client_to_server() {
Expand Down Expand Up @@ -203,68 +202,3 @@ fn server_inactive() {

assert_eq!(app.world().resource::<ServerTick>().get(), 0);
}

#[test]
fn diagnostics() {
let mut server_app = App::new();
let mut client_app = App::new();
for app in [&mut server_app, &mut client_app] {
app.add_plugins((
MinimalPlugins,
RepliconPlugins.set(ServerPlugin {
tick_policy: TickPolicy::EveryFrame,
..Default::default()
}),
))
.replicate::<DummyComponent>();
}
client_app.add_plugins(ClientDiagnosticsPlugin);

server_app.connect_client(&mut client_app);

let client_entity = client_app.world_mut().spawn_empty().id();
let server_entity = server_app
.world_mut()
.spawn((Replicated, DummyComponent))
.id();

let client = client_app.world().resource::<RepliconClient>();
let client_id = client.id().unwrap();

let mut entity_map = server_app.world_mut().resource_mut::<ClientEntityMap>();
entity_map.insert(
client_id,
ClientMapping {
server_entity,
client_entity,
},
);

server_app.world_mut().spawn(Replicated).despawn();

server_app.update();
server_app.exchange_with_client(&mut client_app);
client_app.update();
server_app.exchange_with_client(&mut client_app);

server_app
.world_mut()
.get_mut::<DummyComponent>(server_entity)
.unwrap()
.set_changed();

server_app.update();
server_app.exchange_with_client(&mut client_app);
client_app.update();

let stats = client_app.world().resource::<ClientStats>();
assert_eq!(stats.entities_changed, 2);
assert_eq!(stats.components_changed, 2);
assert_eq!(stats.mappings, 1);
assert_eq!(stats.despawns, 1);
assert_eq!(stats.messages, 2);
assert_eq!(stats.bytes, 33);
}

#[derive(Component, Deserialize, Serialize)]
struct DummyComponent;
Loading