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

Add a configuration to not log commands #10537

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

Conversation

BnDLett
Copy link
Contributor

@BnDLett BnDLett commented Mar 7, 2025

Especially given that some servers or plugins may have commands that they don't want to have logged. Such as this plugin.

If your pull request is not translation or serverlist-related, read the list of requirements below and check each box:

  • I have read the contribution guidelines.
  • I have ensured that my code compiles, if applicable.
  • I have ensured that any new features in this PR function correctly in-game, if applicable.

Especially given that some servers or plugins may have commands that
they don't want to have logged.
@JasonP01
Copy link
Contributor

JasonP01 commented Mar 7, 2025

In CN we used menus with commands as those arent logged to console, very helpful with keeping account login information outside the console logs

@BnDLett
Copy link
Contributor Author

BnDLett commented Mar 7, 2025

In CN we used menus with commands as those arent logged to console, very helpful with keeping account login information outside the console logs

Is there a way to get text from a menu via a server-side plugin? I didn't see a way when I was looking through Call.

@JasonP01
Copy link
Contributor

JasonP01 commented Mar 7, 2025

Is there a way to get text from a menu via a server-side plugin? I didn't see a way when I was looking through Call.

Thats how we do it yes, check out xpdustry/imperium and our AccountCommand.kt file in the mindustry directory, i think thats where login is (you can search for it to)

@JasonP01
Copy link
Contributor

JasonP01 commented Mar 7, 2025

We use distributor framework for our menus but its possible in vanilla with some engineering

@1ue999
Copy link
Contributor

1ue999 commented Mar 8, 2025

In CN we used menus with commands as those arent logged to console, very helpful with keeping account login information outside the console logs

Is there a way to get text from a menu via a server-side plugin? I didn't see a way when I was looking through Call.

you can use Call.textInput (or smth)

@BnDLett
Copy link
Contributor Author

BnDLett commented Mar 8, 2025

That's certainly useful. Danke.

@way-zer
Copy link
Contributor

way-zer commented Mar 12, 2025

If it's command, you can intrecept ChatPacket before commands handle.

This configuration seems useless in most cases, and may be harder to debug.

@BnDLett
Copy link
Contributor Author

BnDLett commented Mar 12, 2025

If it's command, you can intrecept ChatPacket before commands handle.

This configuration seems useless in most cases, and may be harder to debug.

I did something like that with this
https://github.com/mindustry-ddns-net/MindustryDatabase/blob/5b6b10b7aead36bb5b75920ea09227276f760aa9/src/net/ddns/mindustry/database/javaPlugin/commands/CommandHandler.java

It's a huge band-aid and frankensteined solution, but it worked. Granted, I'm now working towards using Call.textInput and converting the plugin to Kotlin.

Either way, having the configuration there is nice for convenience. Also, how would it make it harder to debug?

@way-zer
Copy link
Contributor

way-zer commented Mar 12, 2025

I did something like that with this https://github.com/mindustry-ddns-net/MindustryDatabase/blob/5b6b10b7aead36bb5b75920ea09227276f760aa9/src/net/ddns/mindustry/database/javaPlugin/commands/CommandHandler.java

//intercept chat packet
listenPacket2Server<SendChatMessageCallPacket> { connection, t ->
    if (t.message.contains("<Picture>") && t.message.contains("http")
        || t.message.contains("<District>")
    ) {
        connection.op("forbidden message")
        return@listenPacket2Server false
    }
    return@listenPacket2Server true
}

//lib
/**
 * @param handle return true to call old handler/origin
 */
@ScriptDsl
inline fun <reified T : Packet> Script.listenPacket2Server(crossinline handle: (NetConnection, T) -> Boolean) {
    onEnable {
        val old = getPacketHandle<T>()
        Vars.net.handleServer(T::class.java) { con, p ->
            if (handle(con, p))
                old.get(con, p)
        }
        onDisable {
            Vars.net.handleServer(T::class.java, old)
        }
    }
}

Either way, having the configuration there is nice for convenience. Also, how would it make it harder to debug?

In most cases, the log is useful. If disable the logging and something wrong, it will be hard to anaylze and reproduce.

@BnDLett
Copy link
Contributor Author

BnDLett commented Mar 12, 2025

In most cases, the log is useful. If disable the logging and something wrong, it will be hard to anaylze and reproduce.

The client still stores previous commands. If something did go wrong, then it wouldn't be hard to share the parameters used. Given that it is also a config, logging can be reenabled (if it ends up disabled). Logging commands is also enabled by default, so you'll need to specifically set the configuration to false (or have a plugin do it) for it to not log the commands.

Either way, I personally see no harm in adding it, and, at the end of it day, it's just a configuration.

@way-zer
Copy link
Contributor

way-zer commented Mar 12, 2025

A global configuration for one sercet input is weird. (even one player and short-term).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants