Open SOF3 opened 5 years ago
After all, the server administrator might not want to allow terminal to have full access, since the terminal may be directly exposed to other users (e.g. Multicraft interface).
The console already has all permissions?
@lukeeey that's exactly what I'm saying. Someone who has read/write permission to stdin/stdout does not mean they have full access to the server.
Edit: What I mentioned about log levels in the OP appears to be the result of some misconception. Do not rely on those, and any correction is welcome.
Related to #2543
Putting this afurther, I would propose that we completely remove terminal-related features in PocketMine-MP.phar. Instad, PocketMine just exposes something like an HTTP/WebSocket server. Running start.exe
/./start
(binary) would run php PocketMine-MP.phar
as a subprocess, and connect to the server exposed in the subprocess and provide CLI normally. For detached deployment like Docker, the logs and command interface can be exposed either by exposing a container port or running docker exec -it pocketmine-server pocketminectl
.
On a related note, a cool library I found: https://github.com/AmokHuginnsson/replxx
Implementing CLI as a wrapper would make it easier to use libraries like this, since we wouldn't need to bother about writing PHP interfaces for them.
https://github.com/nushell/reedline also supports features like arrow keys and vi-based cursor motion etc
Introduction
The orientation of logger
The Logger API is, as its name describes, designed for logging. Regular logging includes debug data, progress messages, event logs, abnormal behaviour warning and error messages. However, PocketMine and plugins have vastly abused the Logger as the terminal, gradually mixing interactive interface (such as command response messages) with logging. Moreover, it is impossible to log-without-print or print-without-log. Most processes use stdout as the logging output, but PocketMine outputs to both stdout and server.log, rendering stdout useless.
Interactive terminal interface
While PocketMine reads input from stdin and respond via stdout, terminal experience is unsatisfactory in general. To give a few examples of current limitations:
ob_start
etc.), the interactive terminal display would be very broken.Not to mention that interactive terminal for a server software is... too ancient to say how ancient it is.
Possibility of GUI wrapping
The current distribution requires a semi-interactive terminal as the sole interface (although plugins can start an IPC socket server, some behaviour are not entirely reliable).
Proposal: CLI as a wrapper
The command line interface should only be one of the many possible methods of administration. However, the current code deeply assumes that terminal is the only way of interaction, and uses a lot of hacks to interact in this way.
The correct method is to remove the CLI entirely from the main parts of PocketMine. The main parts of PocketMine should be shipped as a library rather than as the binary itself. This implies that PocketMine-as-a-library cannot assume the existence of a console nor the PocketMine.php entry point. In fact, it should not even assume that there is a wrapper, because terminal in a 24/7 software is pointless -- you won't have a person watching the console 24/7.
Implications
Backward compatibility
This implies that console-related classes, in particular ConsoleCommandSender, would be removed from the core library. Although this might break some plugins, plugins should not have assumed console to be the sole super-user at all, so this is just correcting wrong behaviour. Plugins mostly use ConsoleCommandSender for permission checking, but this should be performed by permission checking. After all, the server administrator might not want to allow terminal to have full access, since the terminal may be directly exposed to other users (e.g. Multicraft interface).
Alert and Emergency
This proposal renders
alert
,emergency
, etc. unreliable for notification. However, currently, these log levels do not beep the console anyway, so this does not change any fact -- alert does not imply user would be alerted. After all, most users won't be next to the console 24/7, so there is no such thing as "emergency". I would suggest that these log levels are misleading and should be removed, but this is another issue. Nevertheless, if a wrapper does support alerting the user, it can attach to the logger anyway.