-
Notifications
You must be signed in to change notification settings - Fork 13
More chat patterns (timestamp and log), take 2 #49
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
base: main
Are you sure you want to change the base?
Changes from all commits
65fff76
dc9bece
de6d75f
7198eb7
2a3afe8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| package ram.talia.moreiotas.api.util; | ||
|
|
||
| public record ChatEntry(String message, long worldTime, String username) { } |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After discussing this on Discord, I think this pattern should be changed to pop nothing from the stack and push a list of lists, where each inner list is For example:
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| package ram.talia.moreiotas.common.casting.actions.strings | ||
|
|
||
| import at.petrak.hexcasting.api.casting.castables.ConstMediaAction | ||
| import at.petrak.hexcasting.api.casting.eval.CastingEnvironment | ||
| import at.petrak.hexcasting.api.casting.getPositiveInt | ||
| import at.petrak.hexcasting.api.casting.iota.DoubleIota | ||
| import at.petrak.hexcasting.api.casting.iota.Iota | ||
| import at.petrak.hexcasting.api.casting.iota.ListIota | ||
| import ram.talia.moreiotas.xplat.IXplatAbstractions | ||
| import ram.talia.moreiotas.api.casting.iota.StringIota as ApiStringIota | ||
|
|
||
| class OpChatLog() : ConstMediaAction { | ||
| override val argc = 1 | ||
|
|
||
| override fun execute(args: List<Iota>, env: CastingEnvironment): List<Iota> { | ||
| val count = args.getPositiveInt(0, argc) | ||
| val logs = IXplatAbstractions.INSTANCE.chatLog(count) | ||
| val messages = mutableListOf<Iota>() | ||
| val timestamps = mutableListOf<Iota>() | ||
| val names = mutableListOf<Iota>() | ||
|
|
||
| for (entry in logs) { | ||
| messages.add(ApiStringIota.make(entry.message)) | ||
| timestamps.add(DoubleIota(entry.worldTime.toDouble())) | ||
| names.add(ApiStringIota.make(entry.username)) | ||
| } | ||
|
|
||
| return listOf(ListIota(messages), ListIota(timestamps), ListIota(names)) | ||
| } | ||
| } |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should return elapsed time instead of an absolute timestamp, and the allChat variant should return |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package ram.talia.moreiotas.common.casting.actions.strings | ||
|
|
||
| import at.petrak.hexcasting.api.casting.castables.ConstMediaAction | ||
| import at.petrak.hexcasting.api.casting.asActionResult | ||
| import at.petrak.hexcasting.api.casting.eval.CastingEnvironment | ||
| import at.petrak.hexcasting.api.casting.iota.DoubleIota | ||
| import at.petrak.hexcasting.api.casting.iota.Iota | ||
| import net.minecraft.world.entity.player.Player | ||
| import ram.talia.moreiotas.api.asActionResult | ||
| import ram.talia.moreiotas.xplat.IXplatAbstractions | ||
|
|
||
| class OpChatTimestamp(private val allChat: Boolean) : ConstMediaAction { | ||
| override val argc = 0 | ||
|
|
||
| override fun execute(args: List<Iota>, env: CastingEnvironment): List<Iota> { | ||
| if (!allChat) | ||
| return IXplatAbstractions.INSTANCE.lastMessageTimestamp(env.castingEntity as? Player).asActionResult | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: this should either be on the same line as the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does this behave in a playerless casting environment (eg. an unbound cleric impetus)? Does it just give the global value? That seems confusing. |
||
| return listOf<Iota>( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: is the type required here? |
||
| DoubleIota(IXplatAbstractions.INSTANCE.lastMessageCount().toDouble()), | ||
| DoubleIota(IXplatAbstractions.INSTANCE.lastMessageTimestamp(null).toDouble()) | ||
| ) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,6 +105,30 @@ | |
| "output": "str", | ||
| "text": "moreiotas.page.strings.string/chat/all" | ||
| }, | ||
| { | ||
| "type": "hexcasting:pattern", | ||
| "op_id": "moreiotas:string/chat/timestamp/caster", | ||
| "anchor": "moreiotas:string/chat/timestamp/caster", | ||
| "input": "", | ||
| "output": "str", | ||
| "text": "moreiotas.page.strings.string/chat/timestamp/caster" | ||
| }, | ||
| { | ||
| "type": "hexcasting:pattern", | ||
| "op_id": "moreiotas:string/chat/timestamp/all", | ||
| "anchor": "moreiotas:string/chat/timestamp/all", | ||
| "input": "", | ||
| "output": "str", | ||
| "text": "moreiotas.page.strings.string/chat/timestamp/all" | ||
| }, | ||
| { | ||
| "type": "hexcasting:pattern", | ||
| "op_id": "moreiotas:string/chat/log/all", | ||
| "anchor": "moreiotas:string/chat/log/all", | ||
| "input": "", | ||
| "output": "str", | ||
| "text": "moreiotas.page.strings.string/chat/log/all" | ||
| }, | ||
|
Comment on lines
+108
to
+131
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The input/output needs to be updated for all of these. |
||
| { | ||
| "type": "hexcasting:pattern", | ||
| "op_id": "moreiotas:string/chat/prefix/set", | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -23,6 +23,9 @@ | |||||
| "hexcasting.action.moreiotas:string/block/set": "Write", | ||||||
| "hexcasting.action.moreiotas:string/chat/caster": "Whisper Reflection", | ||||||
| "hexcasting.action.moreiotas:string/chat/all": "Listener's Reflection", | ||||||
| "hexcasting.action.moreiotas:string/chat/timestamp/caster": "Sandglass' Reflection", | ||||||
| "hexcasting.action.moreiotas:string/chat/timestamp/all": "Clocktower's Reflection", | ||||||
|
Comment on lines
+26
to
+27
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe these could just be called Whisper Reflection II and Listener's Reflection II? I think it would make it more clear that they're returning data about the same event. |
||||||
| "hexcasting.action.moreiotas:string/chat/log/all": "Stenographer's Reflection", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not a reflection if it pops values from the stack. |
||||||
| "hexcasting.action.moreiotas:string/chat/prefix/get": "Sifter's Reflection", | ||||||
| "hexcasting.action.moreiotas:string/chat/prefix/set": "Sifter's Gambit", | ||||||
| "hexcasting.action.moreiotas:string/iota": "Scrivener's Purification", | ||||||
|
|
@@ -90,6 +93,9 @@ | |||||
| "moreiotas.page.strings.string/block/set": "Removes a vector and a string from the stack. If that vector is pointing at a sign or lectern, it writes that string to that sign or lectern. Costs a hundredth of an $(l:items/amethyst)$(item)Amethyst Dust/$.", | ||||||
| "moreiotas.page.strings.string/chat/caster": "Adds the last message the caster sent to the stack as a string.", | ||||||
| "moreiotas.page.strings.string/chat/all": "Adds the last message anyone sent to the stack as a string.", | ||||||
| "moreiotas.page.strings.string/chat/timestamp/caster": "Returns the time in 20ths of a second since the world's beginning, to when the caster sent the last message. Though sometimes it just gives back a zero...", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would make more sense for this to return how much time has elapsed since the message was sent, rather than an absolute timestamp. Also, what does "sometimes it just gives back a zero" mean? |
||||||
| "moreiotas.page.strings.string/chat/timestamp/all": "Similar to $(l:moreiotas:patterns/strings#moreiotas:string/chat/timestamp/caster)$(action)Sandglass' Reflection/$, but tells when anyone sent a message for all to hear. Oddly, returns a count under that too. But it's not like two people would speak at the same single instant, right?", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this return a count? |
||||||
| "moreiotas.page.strings.string/chat/log/all": "All that's spoken loudly enough gets recorded by the Nature, up to a limit. This pattern returns what was said, when, and by whom. The names are not what I see, but what Nature knows people by, though...", | ||||||
|
Comment on lines
+96
to
+98
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These descriptions shouldn't have nearly this much flavour text. Ideally, use the style of the other string patterns as reference - just describe in plain language what the pattern does. |
||||||
| "moreiotas.page.strings.string/chat/prefix/set": "Accepts a string; All future chat messages starting with that string won't be seen by others, and only messages prefixed with that string can be read by $(l:patterns/strings#moreiotas:string/chat/caster)$(action)Whisper Reflection/$.", | ||||||
| "moreiotas.page.strings.string/chat/prefix/get": "Returns the last string you passed to $(l:patterns/strings#moreiotas:string/chat/prefix/set)$(action)Sifter's Gambit/$.", | ||||||
| "moreiotas.page.strings.string/iota": "Converts the iota on top of the stack into a string.", | ||||||
|
|
||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add some comments to this and the Forge equivalent to clarify the control flow and what happens in each case? (eg. sender doesn't have a prefix set, sender has a prefix set but the message doesn't start with it, sender has a prefix and the message starts with it) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,32 +4,69 @@ import net.minecraft.network.chat.ChatType | |
| import net.minecraft.network.chat.PlayerChatMessage | ||
| import net.minecraft.server.level.ServerPlayer | ||
| import net.minecraft.world.entity.player.Player | ||
| import ram.talia.moreiotas.api.mod.MoreIotasConfig.server | ||
| import ram.talia.moreiotas.api.util.ChatEntry | ||
| import ram.talia.moreiotas.fabric.cc.MoreIotasCardinalComponents | ||
| import java.util.* | ||
| import java.util.UUID | ||
| import kotlin.collections.ArrayDeque | ||
|
|
||
| object ChatEventHandler { | ||
| private val lastMessages: MutableMap<UUID, String?> = mutableMapOf() | ||
| private var lastMessage: String? = null | ||
| private val messageLog: ArrayDeque<ChatEntry> = ArrayDeque<ChatEntry>() | ||
| private var lastMessageTimestamp: Long = 0; | ||
| private var messagesHandled: Int = 0; | ||
|
|
||
| private val lastMessageTimestamps: MutableMap<UUID, Long?> = mutableMapOf() | ||
|
|
||
| fun receiveChat(message: PlayerChatMessage, player: ServerPlayer, params: ChatType.Bound): Boolean { | ||
| val text = message.signedBody.content + (message.unsignedContent?.string ?: "") | ||
|
|
||
| val prefix = MoreIotasCardinalComponents.CHAT_PREFIX_HOLDER.get(player).prefix | ||
|
|
||
| var timestamp = player.serverLevel().gameTime | ||
|
|
||
| if (prefix != null && text.startsWith(prefix)) { | ||
| lastMessages[player.uuid] = text.substring(prefix.length) | ||
| lastMessageTimestamps[player.uuid] = timestamp | ||
| return false | ||
| } | ||
|
|
||
| if (prefix == null) { | ||
| lastMessages[player.uuid] = text | ||
| lastMessage = text | ||
| return true | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this removed? |
||
| lastMessageTimestamps[player.uuid] = timestamp | ||
| } | ||
|
|
||
| if (!text.startsWith(prefix)) | ||
| // Just in case! People configure stuff in the weirdest ways | ||
| if (server.maxChatLog == 0) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if it's less than 0? |
||
| return true | ||
|
|
||
| lastMessages[player.uuid] = text.substring(prefix.length) | ||
| while (messageLog.isNotEmpty() && messageLog.size > server.maxChatLog) { | ||
| // Just in case somehow we end up putting two "at once" even though this is only place this is touched | ||
| messageLog.removeFirst() | ||
| } | ||
|
|
||
| messageLog.addLast(ChatEntry(text, timestamp, player.name.string)) | ||
|
Comment on lines
+43
to
+48
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If |
||
|
|
||
| return false | ||
| if (lastMessageTimestamp == timestamp) { | ||
| messagesHandled++ | ||
| } else { | ||
| messagesHandled = 1 | ||
| lastMessageTimestamp = timestamp | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| @JvmStatic | ||
| fun lastMessage(player: Player?): String? = if (player != null) lastMessages[player.uuid] else lastMessage | ||
| fun lastMessage(player: Player?): String? = if (player != null) lastMessages.getOrDefault(player.uuid, null) else if (lastMessageTimestamp != 0L) messageLog.last().message else null | ||
|
|
||
| @JvmStatic | ||
| fun lastMessageTimestamp(player: Player?): Long = (if (player != null) lastMessageTimestamps.getOrDefault(player.uuid, 0) else lastMessageTimestamp) ?: 0 | ||
|
|
||
| @JvmStatic | ||
| fun lastMessageCount(): Int = messagesHandled | ||
|
|
||
| @JvmStatic | ||
| fun chatLog(count: Int): List<ChatEntry> { | ||
| return messageLog.subList((messageLog.size - count).coerceAtLeast(0), messageLog.size).reverse() | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a really weirdly specific default. What about something like 256 instead? Or 64 might be a more reasonably balanced value.