MovingBlocks / FacadeServer

A headless facade that acts as a game host and provides web-based administration. Automation category: Terasology Facade. See https://forum.terasology.org/threads/facadeserver-headless-server-with-web-interface.1906
Apache License 2.0
4 stars 11 forks source link

add a resource for the console help text #24

Closed Inei1 closed 6 years ago

Inei1 commented 6 years ago

This PR adds a resource which simulates calling the help command, but only commands that have the runOnServer boolean are showed in the command list.

see https://github.com/MovingBlocks/FacadeServer-frontend/pull/10

gianluca-nitti commented 6 years ago

(posting here just for reference since we already discussed this on Slack)

The disadvantage of using the solution provided by the combination of this PR + MovingBlocks/FacadeServer-frontend#10 is that it relies on the client. If the user directly queries the API via HTTP/WebSocket and/or builds an alternative client based on this, they'll have to reimplement the same logic added in the frontend PR to get the correct help message. Instead, what I'd do would be changing the POST handler in the Console resource by putting something like this in place of console.execute(data, client.getEntity()):

if (data.startsWith("help")) {
    //your custom help command as implemented here https://github.com/MovingBlocks/FacadeServer/pull/24/files#diff-010aaa8c7c5cc2c054bd8f7932cf5336R44
} else {
    console.execute(data, client.getEntity())
}

I now notice there is a little challenge - how to actually send the built help message, since the client doesn't expect any data as a response to a POST to the console since the response is usually provided by new console messages that the user gets from events generated by the console resource (like here). However, that line (the implementation of onMessage) suggests an easy solution - we could generate a n event here as well. So to expand the previous code snippet:

if (data.startsWith("help")) {
    notifyEvent(client.getEntity(), /* the help message string, built as you already know */);
} else {
    console.execute(data, client.getEntity())
}

IIRC the first argument to notifyEvent should be the client entity used to identify the client to send the notification to, so getting it from the Client instance that originated the request sounds reasonable to it, do you think it would work?

Inei1 commented 6 years ago

The code works fine with being moved to the backend, all I had to do was copy the frontend code and change around the syntax. I had to manually use onMessage() because notifyEvent() was sending an empty message for some reason.

gianluca-nitti commented 6 years ago

Thanks, tested and it's fine, so I'm merging this PR now.

By the way - the reason notifyEvent() with a string doesn't work properly is due to serialization reasons; when I wrote the previous comment, I didn't remember that events are Objects and are serialized right before being inserted in the JSON message to be sent to the client (I thought they were Strings). The Message returned by ConsoleMessageEvent.getFormattedMessage() contains more info than just a string (the other field is the type, which can be console, chat, error, etc) , and the frontend expects the message to be a JSON objects containing the message string and a type.