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

Fix animation timing issues. Reduce heap allocations when loading GFX #306

Merged
merged 15 commits into from May 10, 2023

Conversation

ethanmoffat
Copy link
Owner

This change includes timing updates as well as performance updates to reduce the number of garbage collections happening in the background.

Timing Updates

Completely reworked my timing system to be tick-based like the vanilla client.

Timestamp measurements:

Action Original Client EndlessClient
Walk 48 48
Attack 60 61

Approach:

  • Remove System.Diagnostics.Stopwatch and use monogame's frame update GameTime for all animation timing
  • On each Update() call, increment a global Tick
  • Store Tick value when an animation starts
  • Compare Tick difference when updating animation frames
  • Animation timing based on elapsed EO ticks rather than stopwatch millisecond differences
  • EO timestamp is now just the value of Tick since they're updated every 10ms

The end result is that there's a single place where animation timings are tracked. The behavior is now significantly more consistent and closer to vanilla.

Fixes #299

Memory usage updates

Updated PELoaderLib to use Span<T> and Memory<T> to load data, and removed ImageSharp as a dependency in the texture loading pipeline. This reduces the number of intermediate heap allocations and data copies when converting a texture from a PE resource to a Texture2D. This relies on some unsafe code and pointer arithmetic for high performance and reduced memory footprint

Additionally, found a couple memory hotspots where lots of allocations were happening:

  • A texture was being reloaded from disk in each Draw call in StatusBarLabel
  • NPCs on mouse over were calling GetData in each Update call

StatusBarLabel texture is now just loaded once, and NPCs now use an LRU caching mechanism to store the pixel data of the underlying textures. This has an increased memory footprint, but reduces the number of GetData calls needed, which reduces the number of heap allocations and eventual garbage collections.

This partially addresses #230 but more work still needs to be done

- Main character moving/attacking more slowly than vanilla client
- Other characters moving more slowly than vanilla client, causing weird walk glitches when their walk ended
- Main character timestamps not having high enough resolution
- Main character timestamps not being sent at consistent enough intervals
…ting textures instead of parsing through SixLabors.ImageSharp

Reduces number of transient heap-allocated objects that need to be garbage collected. Reduces data copies and external dependencies.
…on timing issues and makes behavior more consistent with vanilla client
@ethanmoffat ethanmoffat added this to the v1-beta milestone May 9, 2023
@ethanmoffat ethanmoffat merged commit bc2dba5 into master May 10, 2023
3 checks passed
@ethanmoffat ethanmoffat deleted the timings_and_perf branch May 10, 2023 04:38
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.

Walk/Attack animation timings still need work
1 participant