Skip to content

Commit

Permalink
Fix switching within menus calling close handlers causing a cascade
Browse files Browse the repository at this point in the history
Caused child menus to be closed when opening said child menu
  • Loading branch information
Aeltumn committed Jun 3, 2024
1 parent d400919 commit 3c3e2f8
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 20 deletions.
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ val javaVersion: Int = 21

allprojects {
group = "com.noxcrew.interfaces"
version = "1.1.9-SNAPSHOT"
version = "1.1.10"

tasks.withType<JavaCompile> {
sourceCompatibility = javaVersion.toString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public class InterfacesListeners private constructor(private val plugin: Plugin)
val shouldReopen = reason in REOPEN_REASONS && !event.player.isDead && openInterface != null

// Mark the current view as closed properly
view.markClosed(reason, !shouldReopen && reason != Reason.OPEN_NEW)
view.markClosed(reason, shouldReopen || reason == Reason.OPEN_NEW)

// If possible, open back up a previous interface
if (shouldReopen) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ public abstract class AbstractInterfaceBuilder<P : Pane, I : Interface<P>> inter
/** Keeps items that were previously in the inventory before opening the view. */
public var inheritExistingItems: Boolean = false

/** Whether close handlers should be called when switching to a different view. */
public var callCloseHandlerOnViewSwitch: Boolean = true

/** The properties object to use for the created interface. */
public val properties: InterfaceProperties<P>
get() = InterfaceProperties(
Expand All @@ -52,7 +55,8 @@ public abstract class AbstractInterfaceBuilder<P : Pane, I : Interface<P>> inter
preventClickingEmptySlots,
preventedInteractions,
persistAddedItems,
inheritExistingItems
inheritExistingItems,
callCloseHandlerOnViewSwitch
)

/** Adds a new transform to the interface that updates whenever [triggers] change. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,7 @@ public data class InterfaceProperties<P : Pane>(
/** Persists items added to this pane in a previous instance. */
public val persistAddedItems: Boolean = false,
/** Keeps items that were previously in the inventory before opening this. */
public val inheritExistingItems: Boolean = false
public val inheritExistingItems: Boolean = false,
/** Whether close handlers should be called when switching to a different view. */
public val callCloseHandlerOnViewSwitch: Boolean = true
)
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public abstract class AbstractInterfaceView<I : InterfacesInventory, P : Pane>(
/** Marks this menu as closed and processes it. */
internal suspend fun markClosed(
reason: InventoryCloseEvent.Reason = InventoryCloseEvent.Reason.UNKNOWN,
closeInventory: Boolean = reason != InventoryCloseEvent.Reason.OPEN_NEW
changingView: Boolean = reason == InventoryCloseEvent.Reason.OPEN_NEW
) {
// End a possible chat query with the listener
InterfacesListeners.INSTANCE.abortQuery(player.uniqueId, this)
Expand All @@ -93,18 +93,21 @@ public abstract class AbstractInterfaceView<I : InterfacesInventory, P : Pane>(
openIfClosed.set(false)

// Run a generic close handler if it's still opened
if (shouldBeOpened.compareAndSet(true, false)) {
if (shouldBeOpened.compareAndSet(true, false) && (!changingView || backing.properties.callCloseHandlerOnViewSwitch)) {
backing.properties.closeHandlers[reason]?.invoke(reason, this)
}

// Close any children, this is a bit of a lossy system,
// we don't particularly care if this happens nicely we
// just want to make sure the ones that need closing get
// closed. The hashmap is weak so children can get GC'd
// properly.
for ((child) in children) {
if (child.shouldBeOpened.get()) {
child.close(reason, closeInventory)
// Don't close children when changing views!
if (!changingView) {
// Close any children, this is a bit of a lossy system,
// we don't particularly care if this happens nicely we
// just want to make sure the ones that need closing get
// closed. The hashmap is weak so children can get GC'd
// properly.
for ((child) in children) {
if (child.shouldBeOpened.get()) {
child.close(reason, false)
}
}
}
}
Expand Down Expand Up @@ -156,11 +159,12 @@ public abstract class AbstractInterfaceView<I : InterfacesInventory, P : Pane>(
}
}

override suspend fun close(reason: InventoryCloseEvent.Reason, closeInventory: Boolean) {
markClosed(reason, closeInventory)
override suspend fun close(reason: InventoryCloseEvent.Reason, changingView: Boolean) {
markClosed(reason, changingView)

if (isOpen() && closeInventory) {
// Ensure we always close on the main thread!
// Ensure we always close on the main thread! Don't close if we are
// changing views though.
if (!changingView && isOpen()) {
runSync {
player.closeInventory()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ public class PlayerInterfaceView internal constructor(
}
}

override suspend fun close(reason: InventoryCloseEvent.Reason, closeInventory: Boolean) {
markClosed(reason, closeInventory)
override suspend fun close(reason: InventoryCloseEvent.Reason, changingView: Boolean) {
markClosed(reason, changingView)

// Ensure we update the interface state in the main thread!
// Even if the menu is not currently on the screen.
Expand Down

0 comments on commit 3c3e2f8

Please sign in to comment.