Skip to content

feat(ModuleNotifier): Add Module to get feedback in chat when modules toggle#258

Open
IceTank wants to merge 3 commits intolambda-client:1.21.11from
IceTank:feature/module/module-notifyer
Open

feat(ModuleNotifier): Add Module to get feedback in chat when modules toggle#258
IceTank wants to merge 3 commits intolambda-client:1.21.11from
IceTank:feature/module/module-notifyer

Conversation

@IceTank
Copy link
Copy Markdown
Contributor

@IceTank IceTank commented Mar 7, 2026

Adds chat feedback when modules toggle

@IceTank IceTank marked this pull request as ready for review March 7, 2026 21:36
@IceTank IceTank changed the title Add ModuleNotifier to get feedback in chat when modules toggle feat(ModuleNotifier): Add Module to get feedback in chat when modules toggle Mar 7, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in Kanban Mar 11, 2026
@beanbag44 beanbag44 self-requested a review March 12, 2026 14:56
Comment thread src/main/kotlin/com/lambda/util/Communication.kt Outdated
Comment thread src/main/kotlin/com/lambda/module/Module.kt Outdated
Comment thread src/main/kotlin/com/lambda/util/text/TextDsl.kt Outdated
Comment thread src/main/kotlin/com/lambda/util/text/TextDsl.kt Outdated
@IceTank
Copy link
Copy Markdown
Contributor Author

IceTank commented Mar 13, 2026

I refactored the ModuleNotifier so it listens to events produced by the module when toggling

Copy link
Copy Markdown
Member

@beanbag44 beanbag44 left a comment

Choose a reason for hiding this comment

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

I dont know, just assuming but if portions of this pr and others are written with ai, please try to clean up the code (unused imports, old comments, etc), and test it to make sure it has the desired outcome

Comment thread src/main/kotlin/com/lambda/util/Communication.kt Outdated
Comment thread src/main/kotlin/com/lambda/util/Communication.kt Outdated
Comment thread src/main/kotlin/com/lambda/module/Module.kt
Comment thread src/main/kotlin/com/lambda/module/Module.kt
Comment thread src/main/kotlin/com/lambda/module/modules/client/ModuleNotifier.kt Outdated
Comment thread src/main/kotlin/com/lambda/module/modules/client/ModuleNotifier.kt Outdated
Comment thread src/main/kotlin/com/lambda/module/modules/client/ModuleNotifier.kt Outdated
@IceTank IceTank requested a review from beanbag44 April 4, 2026 21:23
Remove redundant toggle listen in ModuleNotifier
Clean up code
Copy link
Copy Markdown
Member

@beanbag44 beanbag44 left a comment

Choose a reason for hiding this comment

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

Im still not big on the idea of module toggles / on / off being cancellable. Maybe just remove that for now because i cant think of a time when that would be necessary. Aside from that as long as this is done its good to merge

}
}

private fun logToTargets(module: Module, message: Text) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could prob be changed to have the message: Text param be a TextBuilder.() -> Unit instead. That way you could have the block outside the param parenthesis

description = "Notifies you when a module is enabled or disabled",
tag = ModuleTag.CLIENT
) {
var notifyTarget by setting("Notify Target", setOf<NotifyTarget>(NotifyTarget.ActionBar), NotifyTarget.entries.toSet(), description = "Where to send notifications when modules are toggled")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i'd add params to the enum rather than using a collection setting here. One boolean for chat, and one for actionBar. Then have a third enum selection for Both, which sets true for both. Then just check if the enum has the value set to true for whatever youre testing

import com.lambda.module.Module

/**
* Represents events related to toggling, enabling, and disabling of [Module]s.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the kt docs here are pretty self explanatory; can honestly just remove them for this class / child classes

@beanbag44
Copy link
Copy Markdown
Member

oh also prob good to make this module enabled by default as its pretty standard across clients

@IceTank IceTank requested a review from beanbag44 April 17, 2026 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants