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

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