Skip to content

Add Chat-Bubbles Feature#29

Open
Wueffi wants to merge 39 commits into
OpenRedstoneEngineers:masterfrom
Wueffi:master
Open

Add Chat-Bubbles Feature#29
Wueffi wants to merge 39 commits into
OpenRedstoneEngineers:masterfrom
Wueffi:master

Conversation

@Wueffi

@Wueffi Wueffi commented Apr 26, 2026

Copy link
Copy Markdown
Member

This PR adds what the title says 🧋

paulikauro
paulikauro requested changes Apr 26, 2026
View reviewed changes

@paulikauro paulikauro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool! One general comment: "Chat-Bubble" -> "chat bubble". Some simplifications and a few things to think about in code comments

Comment thread chattore/src/main/kotlin/feature/BubbleCommand.kt Outdated
Comment thread chattore/src/main/kotlin/feature/BubbleCommand.kt Outdated
Comment thread chattore/src/main/kotlin/feature/BubbleCommand.kt Outdated
Comment thread chattore/src/main/kotlin/feature/BubbleCommand.kt Outdated
Comment thread chattore/src/main/kotlin/feature/BubbleCommand.kt Outdated
Comment thread chattore/src/main/kotlin/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/ChattOREConfig.kt Outdated
Comment thread chattore/src/main/kotlin/Messenger.kt Outdated
Comment thread chattore/src/main/kotlin/Messenger.kt Outdated
Comment thread chattore/src/main/kotlin/Messenger.kt Outdated
@Wueffi Wueffi requested a review from paulikauro April 27, 2026 13:50
paulikauro
paulikauro reviewed Apr 27, 2026
View reviewed changes
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
paulikauro
paulikauro reviewed Apr 27, 2026
View reviewed changes
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
paulikauro
paulikauro requested changes Apr 27, 2026
View reviewed changes

@paulikauro paulikauro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice work! There are some ACF things that would be good to address at this point:

  • commands should take target: OnlinePlayer
  • Bubble should get its own ACF context (like mentioned previously)
  • additionally, the Bubble context should have a flag for checking bubble ownership. Then you can simply do @Flag(FLAG_BUBBLE_OWNED) bubble: Bubble whenever a command needs the sender to be an owner of a bubble (delete, kick, setPrivate). (There FLAG_BUBBLE_OWNED is just a constant string; compare to eg UserCache.COMPLETION_USERNAMES, except this one doesn't have to be public)
    Once implemented, these will make the commands shorter and nicer to read, plus there'll be less duplication :)

Comment thread chattore/src/main/kotlin/Messenger.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
paulikauro
paulikauro reviewed Apr 27, 2026
View reviewed changes
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
@Wueffi Wueffi requested a review from paulikauro May 3, 2026 16:04
@Wueffi

Wueffi commented May 3, 2026

Copy link
Copy Markdown
Member Author

Review requested!

paulikauro
paulikauro requested changes May 3, 2026
View reviewed changes

@paulikauro paulikauro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, some small details still and a few bigger suggestions

Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/ChattOREConfig.kt Outdated
Wueffi and others added 2 commits May 4, 2026 18:28
@Wueffi
Another round of review
7a48000
@paulikauro @Wueffi
Fix handleCommandException
62418d2 
`sender` is never Player, so the else branch was always taken.
Now every error will also get the info prefix added to it.
In the future, a sendError may be useful to add.
@Wueffi Wueffi requested a review from paulikauro May 4, 2026 16:31
@Wueffi

Wueffi commented May 4, 2026

Copy link
Copy Markdown
Member Author

I'd like to formally request a review from the great PaukkuPalikka again

paulikauro
paulikauro reviewed May 4, 2026
View reviewed changes
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/Messenger.kt Outdated
paulikauro
paulikauro reviewed May 4, 2026
View reviewed changes
Comment thread chattore/src/main/kotlin/Messenger.kt Outdated
@Wueffi Wueffi requested a review from paulikauro May 25, 2026 14:47
@Wueffi

Wueffi commented May 25, 2026

Copy link
Copy Markdown
Member Author

I'd like to formally request a review from the great PaukkuPalikka or the great Nickster again

paulikauro
paulikauro reviewed May 25, 2026
View reviewed changes

@paulikauro paulikauro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good! A couple minor things in the comments. I think the only remaining thing we should address is shout going thru the chat filter

Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt Outdated
Comment thread chattore/src/main/kotlin/feature/Bubble.kt
@paulikauro

paulikauro commented May 25, 2026

Copy link
Copy Markdown
Contributor

I forgot shout already did its thing. So the thought here would be to reserve PlayerChatEvent only for players actually typing messages in chat (so no creating and dispatching that event inside the plugin), and instead call whatever functions necessary directly from /shout. That way we don't have to have a set of shouting players and check in the event handler. I think shout should also be in the Bubble file, but it's fine to keep it in Chat for now if it creates dependency problems to move it back into the Bubble file.

@Wueffi Wueffi requested a review from paulikauro May 25, 2026 16:22
Wueffi and others added 26 commits May 27, 2026 18:44
@Wueffi
Reformat code!
ea8e6d4
@paulikauro
Update Gradle (8.14.1 -> 9.5.1)
f56ca1a
@paulikauro
Update Gradle shadow plugin (8.3.6 -> 9.4.2)
e082e7b
@paulikauro
Prevent test task from failing if no tests are discovered
553d3f7
@paulikauro
Add trailing comma
dc971a2
@paulikauro
Add chat listener registration back
c559499
@paulikauro
Remove some type annotations and @single annotations
7741c16
@paulikauro
Use playerOrNull, merge else if
261ca16
@paulikauro
Convert getBubbles() to property instead
061dc6a
@paulikauro
Fix bubble list formatting
becb6fd
@paulikauro
Add hover with bubble info on bubble messages
899c06d
@paulikauro
Convert to expression body
db1de3c
@paulikauro
Tweak bubble messages a little
474ef60
@paulikauro
Add join button to bubble invites
0e1571f
@paulikauro
Hide Discord chat when in bubble and showGlobalChat is off
d40cfd2
@paulikauro
Merge branch 'master'
9c3df33
@paulikauro
Tweak bubble commands and update README accordingly
0461665
@paulikauro
Add bubble command help and command descriptions
26e9637
@paulikauro
Add bubble chat to commandspy
1048382
@paulikauro
Refactor chat confirmations to fix bubble confirms going to global chat
9da2277
@paulikauro
Add sendError and recolor info messages
1d5bfb5
@paulikauro
Prevent confirmations from proceeding when bubble no longer exists
a129464
@paulikauro
Handle global chat confirmations uniformly
f71d89a
@paulikauro
Bump version
9600c18
@paulikauro
Small cleanups
2a383bf
@paulikauro
Move shout back to Bubble.kt and echo message when global chat disabled
32d45b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@paulikauro paulikauro Awaiting requested review from paulikauro

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Wueffi @paulikauro