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.

Sign up for GitHub

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

Throw if module is not setup or cmd missing #64

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Throw if module is not setup or cmd missing #64

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 60 additions & 21 deletions Mage/Module.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,45 @@

namespace Wizcorp.MageSDK.MageClient
{

public class Module<T> : Singleton<T> where T : class, new()
using MageCommandAction = Action<JObject, Action<Exception, JToken>>;
using MageCommandFunction = Func<JObject, UserCommandStatus>;

public abstract class Module<T> : Singleton<T> where T : class, new()
{
//
// Tells us if the module was set up
private bool _SetupCompleted = false;
public bool SetupCompleted {
get { return _SetupCompleted; }
private set { _SetupCompleted = value; }
}


// Mage singleton accessor
protected Mage Mage
{
get { return Mage.Instance; }
}

// Contextualized logger
protected Logger Logger
{
get { return Mage.Logger(GetType().Name); }
}


//
// List of static topics to load during setup
protected virtual List<string> StaticTopics
{
get { return null; }
}

// Static data container
public JToken StaticData;

// Static data setup
//
// Note that topics are not tied to MAGE modules; they are
// essentially global to the MAGE server instance. This is
// simply a convenience function for loading data at setup time.
public void SetupStaticData(Action<Exception> cb)
{
Logger.Info("Setting up static data");
Expand Down Expand Up @@ -67,28 +83,50 @@ public void SetupStaticData(Action<Exception> cb)
}


//
protected virtual string CommandPrefix
{
get { return null; }
}
// The module name as defined on the remote MAGE server
protected abstract string CommandPrefix { get; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, the same as below you're enforcing this to be defined. IIRC the current implementation allows this to remain optional to avoid bloat in modules that have no usercommands.
CC: @nullorvoid

Copy link
Member Author

Choose a reason for hiding this comment

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

The only reason why you would have no user commands is in the case where you only load static data - which is not part of any official MAGE SDK, since MAGE modules do not provide topics.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes but current mage (JS) doesn't enforce these things either
if i have no usercommands I shouldn't have to care about these values
@nullorvoid @kefniark @ronkorving comments welcome

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe the reason why this is not enforced in the JS version is because you can create modules which are meant to be event handlers. Example: https://github.com/mage/mage-sdk-js.session/blob/master/index.js

In our case, since the current module class provides no helpers or support for defining msgStream event hooks, then its only real official purpose appears to be about making user command calls.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, for me this is a choice of how we implement, either I'm fine with.


// The list of available user commands on the remote MAGE server
protected abstract List<string> Commands { get; }

protected virtual List<string> Commands
private Dictionary<string, MageCommandAction> commandHandlerActions;
private Dictionary<string, MageCommandFunction> commandHandlerFuncs;

private void AssertSetupCompleted()
{
get { return null; }
if (SetupCompleted == false)
{
throw new Exception("Module was not setup: " + CommandPrefix);
}
}

private Dictionary<string, Action<JObject, Action<Exception, JToken>>> commandHandlerActions;
private Dictionary<string, Func<JObject, UserCommandStatus>> commandHandlerFuncs;
private M GetCommand<M>(Dictionary<string, M> list, string commandName) {
AssertSetupCompleted();

if (list == null) {
throw new Exception("Module does not define any user commands: " + CommandPrefix);
}

var command = list[commandName];

if (command == null)
{
throw new Exception("User command not found: " + CommandPrefix + "." + commandName);
}

return command;
}

public void Command(string commandName, JObject arguments, Action<Exception, JToken> cb)
{
commandHandlerActions[commandName](arguments, cb);
var action = GetCommand(commandHandlerActions, commandName);
action(arguments, cb);
}

public UserCommandStatus Command(string commandName, JObject arguments)
{
return commandHandlerFuncs[commandName](arguments);
var action = GetCommand(commandHandlerFuncs, commandName);
return action(arguments);
}

private void RegisterCommand(string command)
Expand Down Expand Up @@ -123,20 +161,21 @@ public void SetupUsercommands(Action<Exception> cb)
{
Logger.Info("Setting up usercommands");

commandHandlerActions = new Dictionary<string, Action<JObject, Action<Exception, JToken>>>();
commandHandlerFuncs = new Dictionary<string, Func<JObject, UserCommandStatus>>();

if (Commands == null)
{
if (Commands == null) {
SetupCompleted = true;
cb(null);
return;
}

commandHandlerActions = new Dictionary<string, Action<JObject, Action<Exception, JToken>>>();
commandHandlerFuncs = new Dictionary<string, Func<JObject, UserCommandStatus>>();

foreach (string command in Commands)
{
RegisterCommand(command);
}

SetupCompleted = true;
cb(null);
}
}
Expand Down