Skip to content
This repository has been archived by the owner on Feb 25, 2020. It is now read-only.

Prototype code for packet handler decorators #40

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

snarlynarwhal
Copy link
Contributor

@snarlynarwhal snarlynarwhal commented Mar 23, 2019

I wanted to get feedback before continuing with the last step of wiring up the decorators.

I added 3 classes to Networker:

  • DecorateAttribute - Helper attribute to assign decorators to handlers
    • Example: Decorate[typeof(AuthorizationHandlerDecorator)]
  • DecoratorAttribute - Base attribute class for making custom attributes to assign decorators
    • Example: [Authorize]
  • PacketHandlerDecorator

The attributes can get added to handlers and modules.

I put together Demo.Decorators to demonstrate how the features gets used. It does not function yet.

The last part I need to do is wire up the decorators on startup. Should do this in BuilderBase.SetupSharedDependencies?


namespace Networker.Common
{
public abstract class PacketHandlerDecorator<T> : PacketHandlerBase<T> where T: class
Copy link
Owner

Choose a reason for hiding this comment

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

This particular way of doing it would work well as an extension project, but I would like to avoid the dependency on PacketHandlerBase as you are able to use just an IPacketHandler now.

Would this also mean you'd need a new PacketHandlerDecorator for each packet type?

Copy link
Contributor Author

@snarlynarwhal snarlynarwhal Mar 24, 2019

Choose a reason for hiding this comment

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

Yeah implementing IPacketHandler makes more sense. The decorator has helper methods to get the target handler method info based on the packet type. This assumes the handler methods accept a base packet type though. I definitely need to think more about this.

Where should I wire up the decorators? Once I get it to a testable state, I can play around with ideas to make sure a single decorator can handle multiple packet types.

Copy link
Owner

Choose a reason for hiding this comment

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

I've got a new way of registering packets + handlers coming in soon which might be the best place to wire up the decorator, I'll let you know when it's ready.

Copy link
Contributor Author

@snarlynarwhal snarlynarwhal Mar 27, 2019

Choose a reason for hiding this comment

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

Alright sounds good! Also, it might be preferable to assign decorators using the builder instead of attributes. That approach would require a fluent interface instead of method chaining, which I can add later on if you think it's a good idea.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants