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

WIP: Threading and Plugin System Rewrite #182

Merged
merged 20 commits into from
Nov 10, 2024
Merged

Conversation

tristanpoland
Copy link
Member

@tristanpoland tristanpoland commented Nov 6, 2024

Pull Request Summary: Threading and Plugin System Rewrite

Overview

This PR introduces a major rewrite of the plugin system architecture with a focus on improved threading and state management.

Key Changes

Architecture Changes

  • Complete rewrite of plugin API architecture
  • Introduction of new plugin state management system
  • Migration from Mutex to RwLock for improved concurrent access
  • New threading model with parallel iterators using Rayon

Plugin System Rewrite

This PR also introduces a simplified and more maintainable plugin architecture that replaces the previous trait-heavy implementation.

Key Changes

  • Reduced complexity by removing redundant trait hierarchies
  • Introduced a simple PluginManager with explicit state management (ACTIVE/INACTIVE/CRASH)
  • Added type-safe plugin access through a macro-based system
  • Simplified plugin loading with direct socket and player state injection
  • Implemented version tracking for API compatibility

New Features

  • Clean plugin lifecycle management with explicit states
  • Type-safe plugin access via get_plugin! macro
  • Streamlined plugin loading system
  • Version compatibility checking
  • Automatic plugin discovery and loading through build.rs

Dependencies

  • Updated core dependencies:
    • socketioxide -> 0.15.1
    • Added rayon for parallel processing
    • Removed several unused dependencies
    • Consolidated dependency versions

Performance Optimizations

  • Implemented parallel iterators for player data processing
  • Improved concurrent access patterns using RwLock
  • Better memory management in player state updates

Breaking Changes

  • Plugin API interface has changed significantly
  • State management system requires plugin updates
  • Threading model changes may require client adaptations

Testing

  • Added unit tests for plugin management
  • New test cases for plugin states
  • Error handling test coverage

Migration Notes

Existing plugins will need to be updated to work with the new API structure and state management system.

@tristanpoland tristanpoland added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed performance This item is related to performance (Combine with tags like Bug, Enhancement, etc) Required for other issues dependencies Pull requests that update a dependency file rust Pull requests that update Rust code labels Nov 6, 2024
@tristanpoland tristanpoland self-assigned this Nov 6, 2024
@tristanpoland tristanpoland changed the title Threading and Plugin System Rewrite WIP: Threading and Plugin System Rewrite Nov 6, 2024
@tristanpoland
Copy link
Member Author

This PR among other things will allow us to complete #173 and further integrate with other plugins and game engines over socketioxide

@tristanpoland
Copy link
Member Author

@Tuafo When you get the chance would you mind helping me on a PebbleVault plugin rework? it and a bunch of the other plugins will need to change how they work in order to work properly in the new API. Shouldn't take too long I think.

@tristanpoland
Copy link
Member Author

tristanpoland commented Nov 7, 2024

@haywoodspartan @Tuafo @WilliamAnimate Would you guys mind taking a look at how plugins are written on this branch (mainly the struct plugin defined in the horizon_plugin_api crate and implementing a different trait on the struct in each plugin (see /plugins/test_plugin))

I want to know if you guys think this is a solid way of doing function calls to plugins in a type-safe way.
Also give it a test on your systems and let me know if it seems to work alright

Thanks you guys ❤️

@tristanpoland tristanpoland requested a review from CatTrim November 7, 2024 19:59
@a-catgirl-dev
Copy link
Member

I'll take a look when i get back home; I'm in a class right now

@tristanpoland
Copy link
Member Author

I'll take a look when i get back home; I'm in a class right now

Stay in school kids 😆
Talk to you when you are home then

Copy link
Member

@a-catgirl-dev a-catgirl-dev left a comment

Choose a reason for hiding this comment

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

I see the code

// Define the trait properly
pub trait Plugin_API {    
    fn thing(&self) -> String;
}

pub trait Plugin_Construct {
    // If you want default implementations, mark them with 'default'
    fn new(plugins: HashMap<&'static str, LoadedPlugin>, socket: SocketRef, players: Arc<RwLock<Vec<horizon_data_types::Player>>>) -> Plugin;
}

being repeated multiple times throughout different plugins. maybe put it into some crate, and reference it?

@tristanpoland
Copy link
Member Author

tristanpoland commented Nov 7, 2024

I see the code

// Define the trait properly
pub trait Plugin_API {    
    fn thing(&self) -> String;
}

pub trait Plugin_Construct {
    // If you want default implementations, mark them with 'default'
    fn new(plugins: HashMap<&'static str, LoadedPlugin>, socket: SocketRef, players: Arc<RwLock<Vec<horizon_data_types::Player>>>) -> Plugin;
}

being repeated multiple times throughout different plugins. maybe put it into some crate, and reference it?

This is due to each plugin needing to impl its own API, these can be customized but in the case of the plugins we have it is not yet. Each plugin impl's its own trait onto the Plugin struct. When a fn is called the getplugin! macro type-casts to the proper trait for that plugin allowing type-safe generic function calls to any plugin in the HashMap

@tristanpoland tristanpoland dismissed a-catgirl-dev’s stale review November 10, 2024 17:12

Only applies to current plugins' inplementations, the signatures of functions will very and cannot be centralized unfortunatelty

@tristanpoland tristanpoland removed the request for review from haywoodspartan November 10, 2024 17:12
@tristanpoland tristanpoland marked this pull request as ready for review November 10, 2024 17:14
@tristanpoland tristanpoland merged commit 6f2c852 into main Nov 10, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed performance This item is related to performance (Combine with tags like Bug, Enhancement, etc) Required for other issues rust Pull requests that update Rust code
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants