Skip to content
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

Hide extra functionality from ServerEventQueue #164

Merged
merged 3 commits into from
Jan 10, 2024
Merged

Conversation

Shatur
Copy link
Contributor

@Shatur Shatur commented Jan 10, 2024

In discord we discussed customization without the full access to ServerEventQueue.

@Shatur Shatur force-pushed the server-event-queue branch from f5d08f3 to e3a6fe5 Compare January 10, 2024 11:24
@Shatur Shatur requested a review from UkoeHB January 10, 2024 11:24
@Shatur Shatur force-pushed the server-event-queue branch from e3a6fe5 to 6c99614 Compare January 10, 2024 11:25
@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a502a7d) 83.37% compared to head (eb99af1) 90.68%.
Report is 3 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #164      +/-   ##
==========================================
+ Coverage   83.37%   90.68%   +7.31%     
==========================================
  Files          20       19       -1     
  Lines        1287     1181     -106     
==========================================
- Hits         1073     1071       -2     
+ Misses        214      110     -104     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

/// Inserts a new event.
///
/// The event will be queued until [`Self::pop_next`] is called with a replicon tick >= the tick specified here,
/// or until [`Self::clear`] is called.
/// The event will be queued until [`RepliconTick`] will be bigger or equal to the tick specified here.
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
/// The event will be queued until [`RepliconTick`] will be bigger or equal to the tick specified here.
/// The event will be queued until [`RepliconTick`] is bigger or equal to the tick specified here.

Comment on lines 362 to 369
/// Pops the next event that is at least as old as the specified replicon tick.
pub fn pop_next(&mut self, replicon_tick: RepliconTick) -> Option<T> {
let (tick, _) = self.0.front()?;
if *tick > replicon_tick {
return None;
}
self.0.pop_front().map(|(_, event)| event)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to keep this and just change the visibility, to isolate implementation details from other parts of the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, but I adjusted the name a little bit.

@Shatur Shatur requested a review from UkoeHB January 10, 2024 21:30
Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>
@Shatur Shatur merged commit 50d57c9 into master Jan 10, 2024
5 checks passed
@Shatur Shatur deleted the server-event-queue branch January 10, 2024 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants