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

Better Command Action Scheduling System #51

Open
FlashyReese opened this issue Jan 25, 2023 · 2 comments
Open

Better Command Action Scheduling System #51

FlashyReese opened this issue Jan 25, 2023 · 2 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed

Comments

@FlashyReese
Copy link
Owner

FlashyReese commented Jan 25, 2023

The issue at hand:

private int scheduleAction(Queue<CustomCommandAction> customCommandActionQueue, long triggerTime, CommandDispatcher<S> dispatcher, CommandContext<S> context, List<String> currentInputList) {
AtomicInteger state = new AtomicInteger();
if (customCommandActionQueue.isEmpty()) return Command.SINGLE_SUCCESS;
CustomCommandAction action = customCommandActionQueue.poll();
String eventName = "generic";
if (action.getId() != null && !action.getId().isEmpty())
eventName = this.formatString(context, currentInputList, action.getId());
if (action.getStartTime() != null && !action.getStartTime().isEmpty()) {
String time = this.formatString(context, currentInputList, action.getStartTime());
triggerTime = System.currentTimeMillis() + Long.parseLong(time);
}
this.abstractCommandAliasesProvider.getScheduler().addEvent(new Scheduler.Event(triggerTime, eventName, () -> {
if (action.getCommand() != null && action.getCommandType() != null) {
long startFormat = System.nanoTime();
String actionCommand = this.formatString(context, currentInputList, action.getCommand());
long endFormat = System.nanoTime();
try {
state.set(this.dispatcherExecute(action, dispatcher, context, actionCommand));
} catch (CommandSyntaxException e) {
if (CommandAliasesMod.options().debugSettings.debugMode) {
CommandAliasesMod.logger().error("""
\n\t======================================================
\tFailed to process command
\tOriginal Action Command: {}
\tOriginal Action Command Type: {}
\tPost Processed Action Command: {}
\t======================================================""", action.getCommand(), action.getCommandType(), actionCommand);
String output = e.getLocalizedMessage();
this.sendFeedback(context, output);
}
e.printStackTrace();
}
long endExecution = System.nanoTime();
if (CommandAliasesMod.options().debugSettings.showProcessingTime) {
CommandAliasesMod.logger().info("""
\n\t======================================================
\tOriginal Action Command: {}
\tOriginal Action Command Type: {}
\tPost Processed Action Command: {}
\tFormatting time: {}ms
\tExecuting time: {}ms
\t======================================================""", action.getCommand(), action.getCommandType(), actionCommand, (endFormat - startFormat) / 1000000.0, (endExecution - endFormat) / 1000000.0);
}
if (state.get() != Command.SINGLE_SUCCESS) {
if (action.getMessageIfUnsuccessful() != null) {
String message = this.formatString(context, currentInputList, action.getMessageIfUnsuccessful());
this.sendFeedback(context, message);
}
if (action.getActionsIfUnsuccessful() != null && !action.getActionsIfUnsuccessful().isEmpty()) {
Queue<CustomCommandAction> unsuccessfulActionsQueue = new LinkedList<>(action.getActionsIfUnsuccessful());
state.set(this.scheduleAction(unsuccessfulActionsQueue, System.currentTimeMillis(), dispatcher, context, currentInputList));
}
if (action.isRequireSuccess()) {
customCommandActionQueue.clear();
}
} else {
if (action.getMessageIfSuccessful() != null) {
String message = this.formatString(context, currentInputList, action.getMessageIfSuccessful());
this.sendFeedback(context, message);
}
if (action.getActionsIfSuccessful() != null && !action.getActionsIfSuccessful().isEmpty()) {
Queue<CustomCommandAction> successfulActionsQueue = new LinkedList<>(action.getActionsIfSuccessful());
state.set(this.scheduleAction(successfulActionsQueue, System.currentTimeMillis(), dispatcher, context, currentInputList));
}
}
}
if (action.getMessage() != null) {
String message = this.formatString(context, currentInputList, action.getMessage());
this.sendFeedback(context, message);
}
state.set(this.scheduleAction(customCommandActionQueue, System.currentTimeMillis(), dispatcher, context, currentInputList));
}));
return state.get();
}

The current implementation of the custom command builder method is less efficient than the previous version. The previous version utilized a new thread to call variables, but this caused issues with certain mods like Immersive Portals. The current version uses a scheduler for commands, but this approach has a drawback in that it does not wait for custom commands to finish before scheduling the next one. This can lead to issues with sub-actions not completing before the next action is scheduled.

This is a problem because we use an atomic integer to return the state of our custom commands when we call them within command actions. When the scheduler is used, it can interfere with the state of our data if we are working with the database, leading to unexpected results.

We are seeking suggestions for alternative methods of implementation that do not involve summoning new threads. Feedback on this issue is greatly appreciated.

image
(An outdated image sketch that inadequately demonstrates the current issue)

@FlashyReese FlashyReese added bug Something isn't working enhancement New feature or request help wanted Extra attention is needed labels Jan 25, 2023
@FlashyReese FlashyReese self-assigned this Jan 25, 2023
@FlashyReese
Copy link
Owner Author

To better illustrate the issue, I have created a custom command using an older version of the software. The command is structured as shown below:

{
  "commandMode": "COMMAND_CUSTOM",
  "customCommand" : {
    "parent": "example",
    "actions": [
      {
        "command": "say action 1",
        "commandType": "SERVER",
        "successfulActions": [
          {
            "command": "say action1 subaction 1",
            "commandType": "SERVER",
            "requireSuccess": true
          },
          {
            "command": "say action1 subaction 2",
            "commandType": "SERVER"
          }
        ],
        "unsuccessfulActions": [
          {
            "command": "say action1 subaction 1 failed",
            "commandType": "SERVER",
            "requireSuccess": true
          },
          {
            "command": "say action1 subaction 2 failed",
            "commandType": "SERVER"
          }
        ]
      },
      {
        "command": "say action 3",
        "commandType": "SERVER"
      }
    ]
  }
}

When the command is run, it produces the following output:
image

To work around this implementation issue, one solution is to add a delay. However, this is not ideal as we want our code to be as fast as possible. An example of this workaround is shown below:

{
  "commandMode": "COMMAND_CUSTOM",
  "customCommand" : {
    "parent": "example",
    "actions": [
      {
        "command": "say action 1",
        "commandType": "SERVER",
        "successfulActions": [
          {
            "command": "say action1 subaction 1",
            "commandType": "SERVER",
            "requireSuccess": true
          },
          {
            "command": "say action1 subaction 2",
            "commandType": "SERVER"
          }
        ],
        "unsuccessfulActions": [
          {
            "command": "say action1 subaction 1 failed",
            "commandType": "SERVER",
            "requireSuccess": true
          },
          {
            "command": "say action1 subaction 2 failed",
            "commandType": "SERVER"
          }
        ]
      },
      {
        "triggerTime": "50",
        "command": "say action 3",
        "commandType": "SERVER"
      }
    ]
  }
}

@FlashyReese FlashyReese pinned this issue Jan 25, 2023
@FlashyReese
Copy link
Owner Author

One idea that comes to my mind is replacing/refactoring the scheduler system to use CompletableFuture. This should eliminate certain pitfalls like needing to wait for command return states.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant