Escape on the realm list closes only the list, as in 1.12.1 (#827)
Escape on the realm list closed the list and then also quit the game from the login screen, or went back to login from character select. 1.12.1 walks the keyboard frames once per press, strata high to low, and stops at the first handler that takes it (`0x765f10`, leaving at `0x765f64`/`0x765f86`). The realm list is DIALOG strata (`RealmList.xml:246`), so its Escape never reaches the screen beneath, even though it hides itself. benilla's list and screens read the keys in no fixed order. A screen only checked whether the list was still shown, and the list's own Escape had just hidden it. Who owns a frame's keys is now decided once, before any of them runs (`Realms::owns_input`, set in `PreUpdate`). Tests run the real list and screen systems in both orders, and fail before the fix with the reported symptoms. Fixes #817 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
ea7e0d087a
commit
1c5a568191
@@ -41,7 +41,7 @@ pub(super) fn select_input(
|
||||
) {
|
||||
// A modal owns the input while it is up: the delete confirm, the AddOns list, the realm list
|
||||
// over this screen, or the glue dialog (whose Okay must not double as Enter World or Escape).
|
||||
if dialog.open || panel.open || realms.shown || glue_dialog.is_open() {
|
||||
if dialog.open || panel.open || realms.owns_input() || glue_dialog.is_open() {
|
||||
return;
|
||||
}
|
||||
let now = time.elapsed_secs();
|
||||
@@ -169,3 +169,78 @@ pub(super) fn rotate_model(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::net::{RealmChoice, RealmRequest};
|
||||
use crate::realm_select::{list_keys, take_input, Realms};
|
||||
|
||||
/// Escape on the realm list raised by Change Realm closes the list and nothing else: the
|
||||
/// press is the list's (`Realms::owns_input`), so this screen does not also go back to
|
||||
/// login, in either order the two run.
|
||||
#[test]
|
||||
fn escape_on_the_realm_list_does_not_also_leave_for_login() {
|
||||
for list_first in [true, false] {
|
||||
let (pick_tx, pick_rx) = crossbeam_channel::unbounded();
|
||||
let (realm_tx, realm_rx) = crossbeam_channel::unbounded();
|
||||
let mut app = App::new();
|
||||
app.add_plugins((MinimalPlugins, bevy::state::app::StatesPlugin))
|
||||
.insert_state(ClientState::CharSelect)
|
||||
.init_resource::<crate::glue::GlueClicks>()
|
||||
.init_resource::<ButtonInput<KeyCode>>()
|
||||
.init_resource::<Roster>()
|
||||
.init_resource::<Realms>()
|
||||
.init_resource::<DeleteDialog>()
|
||||
.init_resource::<super::super::addons::AddonsPanel>()
|
||||
.init_resource::<crate::login::LoginIntent>()
|
||||
.init_resource::<crate::glue::dialog::GlueDialog>()
|
||||
.insert_resource(CharPick(pick_tx))
|
||||
.insert_resource(RealmChoice(realm_tx))
|
||||
.add_message::<GlueSound>()
|
||||
.add_message::<bevy::input::mouse::MouseWheel>()
|
||||
.add_systems(PreUpdate, take_input);
|
||||
if list_first {
|
||||
app.add_systems(Update, (list_keys, select_input).chain());
|
||||
} else {
|
||||
app.add_systems(Update, (select_input, list_keys).chain());
|
||||
}
|
||||
app.world_mut()
|
||||
.resource_scope(|world, mut realms: Mut<Realms>| {
|
||||
crate::realm_select::open_over_char_select(
|
||||
&mut realms,
|
||||
world.resource::<RealmChoice>(),
|
||||
);
|
||||
});
|
||||
app.world_mut()
|
||||
.resource_mut::<ButtonInput<KeyCode>>()
|
||||
.press(KeyCode::Escape);
|
||||
app.update();
|
||||
app.world_mut()
|
||||
.resource_mut::<ButtonInput<KeyCode>>()
|
||||
.clear();
|
||||
app.update(); // `StateTransition` applies a pending state at the next frame
|
||||
|
||||
let order = if list_first {
|
||||
"list first"
|
||||
} else {
|
||||
"screen first"
|
||||
};
|
||||
assert!(
|
||||
realm_rx
|
||||
.try_iter()
|
||||
.any(|r| matches!(r, RealmRequest::Abandon)),
|
||||
"{order}: the list's Cancel ran"
|
||||
);
|
||||
assert_eq!(
|
||||
*app.world().resource::<State<ClientState>>().get(),
|
||||
ClientState::CharSelect,
|
||||
"{order}: the press was the list's, so the screen under it stays"
|
||||
);
|
||||
assert!(
|
||||
pick_rx.try_recv().is_err(),
|
||||
"{order}: and the session is not dropped"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -664,8 +664,9 @@ fn login_input(
|
||||
time: Res<Time>,
|
||||
) {
|
||||
// The reference's `RealmList` is a DIALOG-strata frame over this screen, so while it is up
|
||||
// everything here is inert, Escape included (it is the list's Cancel).
|
||||
if realms.shown {
|
||||
// everything here is inert, Escape included (it is the list's Cancel), through the frame
|
||||
// its own Escape closes it in.
|
||||
if realms.owns_input() {
|
||||
return;
|
||||
}
|
||||
let empty = GlueStrings::default();
|
||||
@@ -1006,6 +1007,73 @@ mod tests {
|
||||
(app, rx)
|
||||
}
|
||||
|
||||
/// Escape on the realm list over this screen closes the list and nothing else: the press is
|
||||
/// the list's (`Realms::owns_input`), so the login screen does not also quit, in either
|
||||
/// order the two run.
|
||||
#[test]
|
||||
fn escape_on_the_realm_list_does_not_also_quit() {
|
||||
for list_first in [true, false] {
|
||||
let (tx, _requests) = crossbeam_channel::unbounded();
|
||||
let (realm_tx, realm_rx) = crossbeam_channel::unbounded();
|
||||
let mut app = App::new();
|
||||
app.add_plugins(MinimalPlugins)
|
||||
.init_resource::<crate::glue::GlueClicks>()
|
||||
.init_resource::<ButtonInput<KeyCode>>()
|
||||
.init_resource::<crate::bindings::LayoutNames>()
|
||||
.insert_non_send_resource(HostClipboard::default())
|
||||
.init_resource::<LoginForm>()
|
||||
.init_resource::<LoginIntent>()
|
||||
.init_resource::<GlueDialog>()
|
||||
.init_resource::<crate::realm_select::Realms>()
|
||||
.insert_resource(crate::realmlist::Realmlist::unpinned(
|
||||
crate::realmlist::DEFAULT_REALMLIST,
|
||||
))
|
||||
.insert_resource(LoginSubmit(tx))
|
||||
.insert_resource(LoginAbandon(std::sync::Arc::new(
|
||||
std::sync::atomic::AtomicU64::new(0),
|
||||
)))
|
||||
.insert_resource(crate::net::RealmChoice(realm_tx))
|
||||
.add_message::<KeyboardInput>()
|
||||
.add_message::<GlueSound>()
|
||||
.add_message::<bevy::input::mouse::MouseWheel>()
|
||||
.add_systems(PreUpdate, crate::realm_select::take_input);
|
||||
let list = crate::realm_select::list_keys;
|
||||
if list_first {
|
||||
app.add_systems(Update, (list, login_input).chain());
|
||||
} else {
|
||||
app.add_systems(Update, (login_input, list).chain());
|
||||
}
|
||||
// The list stands over this screen, as the realm park raises it after logon.
|
||||
app.world_mut()
|
||||
.resource_mut::<crate::realm_select::Realms>()
|
||||
.shown = true;
|
||||
app.world_mut()
|
||||
.resource_mut::<ButtonInput<KeyCode>>()
|
||||
.press(KeyCode::Escape);
|
||||
app.update();
|
||||
|
||||
let order = if list_first {
|
||||
"list first"
|
||||
} else {
|
||||
"screen first"
|
||||
};
|
||||
assert!(
|
||||
!app.world().resource::<crate::realm_select::Realms>().shown,
|
||||
"{order}: the list's Cancel closed it"
|
||||
);
|
||||
assert!(
|
||||
realm_rx
|
||||
.try_iter()
|
||||
.any(|r| matches!(r, crate::net::RealmRequest::Abandon)),
|
||||
"{order}: and answered the park"
|
||||
);
|
||||
assert!(
|
||||
!app.world().contains_resource::<QuitArm>(),
|
||||
"{order}: the press was the list's, so the login screen does not also quit"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A lost session does not log itself back in: a displaced client's resubmit would kick the
|
||||
/// client that displaced it. `GlueParent.lua` retries nothing.
|
||||
#[test]
|
||||
|
||||
@@ -68,7 +68,7 @@ pub(super) fn clicks(
|
||||
}
|
||||
|
||||
/// `RealmList_OnKeyDown` (ESCAPE / ENTER), plus arrow-key row cycling and the wheel.
|
||||
pub(super) fn keys(
|
||||
pub(crate) fn keys(
|
||||
keys: Res<ButtonInput<KeyCode>>,
|
||||
(mut wheel, mut wheel_carry): (MessageReader<MouseWheel>, Local<WheelNotches>),
|
||||
mut realms: ResMut<Realms>,
|
||||
|
||||
@@ -25,6 +25,10 @@ mod load;
|
||||
mod screen;
|
||||
mod smoke;
|
||||
|
||||
/// The list's own keys, for a screen's test to run beside its own input.
|
||||
#[cfg(test)]
|
||||
pub(crate) use input::keys as list_keys;
|
||||
|
||||
use bevy::prelude::*;
|
||||
|
||||
use crate::net::{RealmChoice, RealmListMessage, RealmRequest};
|
||||
@@ -65,6 +69,9 @@ pub(crate) struct Realms {
|
||||
/// `RealmList:IsVisible()`. While set, a published list is a refresh and is never
|
||||
/// auto-answered.
|
||||
pub(super) shown: bool,
|
||||
/// [`Self::shown`] as this frame's input found it, taken once in `PreUpdate` by
|
||||
/// [`take_input`]; read through [`Self::owns_input`].
|
||||
owns_input: bool,
|
||||
/// `GetRealmInfo`'s `currentRealm`, the row highlighted when nothing else is. The reference
|
||||
/// case-folds the `realmName` CVar (`0x46ef9e`, via `0x5ab7d0` on the handle from `0x63db90`)
|
||||
/// against the realm name, and `ConnectToRealm` (`0x46b217`) writes the CVar before it dials.
|
||||
@@ -127,6 +134,19 @@ impl Sort {
|
||||
}
|
||||
|
||||
impl Realms {
|
||||
/// Whether this frame's keys and clicks are the list's, not the screen's beneath it.
|
||||
///
|
||||
/// The reference walks a key-down once, strata 8 down to 0 (`0x765f10`), and the first frame
|
||||
/// whose `OnKeyDown` slot returns nonzero ends the walk (`0x765f86`, then `0x765f64`), whatever
|
||||
/// that handler did. `RealmList` is a keyboard-enabled `DIALOG` frame with `OnKeyDown` bound,
|
||||
/// so while it is shown `0x76b7d0` consumes every key it is reached with, ahead of
|
||||
/// `AccountLogin` and `CharacterSelect`, which set no strata. So a press has one owner even
|
||||
/// when its handler hides the list: the list and the screens ask this, never [`Self::shown`],
|
||||
/// which the list's own Escape or Enter clears mid-frame.
|
||||
pub(crate) fn owns_input(&self) -> bool {
|
||||
self.owns_input
|
||||
}
|
||||
|
||||
/// The rows the screen draws: indices into [`Self::realms`] for the tab in front, sorted per
|
||||
/// category (`0x46e750`).
|
||||
pub(super) fn rows(&self) -> Vec<usize> {
|
||||
@@ -243,12 +263,22 @@ pub(super) fn is_invalid(realm: &RealmInfo) -> bool {
|
||||
realm.flags & 0x01 != 0
|
||||
}
|
||||
|
||||
/// Settle who owns this frame's input before the list or any glue screen reads it
|
||||
/// ([`Realms::owns_input`]).
|
||||
pub(crate) fn take_input(mut realms: ResMut<Realms>) {
|
||||
if realms.owns_input != realms.shown {
|
||||
realms.owns_input = realms.shown;
|
||||
}
|
||||
}
|
||||
|
||||
/// The realm-selection subsystem: the park's policy plus the screen.
|
||||
pub(crate) struct RealmSelectPlugin;
|
||||
|
||||
impl Plugin for RealmSelectPlugin {
|
||||
fn build(&self, app: &mut App) {
|
||||
app.init_resource::<Realms>()
|
||||
// `shown` is written only in `Update`, so any slot here sees the frame's start.
|
||||
.add_systems(PreUpdate, take_input)
|
||||
.add_systems(
|
||||
Startup,
|
||||
(category::load_categories, load::load_realm_types)
|
||||
@@ -263,13 +293,14 @@ impl Plugin for RealmSelectPlugin {
|
||||
// Input before the row refresh, so a click shows on the frame it landed.
|
||||
smoke::debug_realm_smoke,
|
||||
(
|
||||
input::clicks,
|
||||
input::keys,
|
||||
tick_refresh,
|
||||
screen::refresh_list,
|
||||
(input::clicks, input::keys)
|
||||
.chain()
|
||||
.run_if(|realms: Res<Realms>| realms.owns_input()),
|
||||
(tick_refresh, screen::refresh_list)
|
||||
.chain()
|
||||
.run_if(|realms: Res<Realms>| realms.shown),
|
||||
)
|
||||
.chain()
|
||||
.run_if(|realms: Res<Realms>| realms.shown),
|
||||
.chain(),
|
||||
)
|
||||
.chain()
|
||||
.after(benilla_world::schedule::WorldStage::Net)
|
||||
|
||||
Reference in New Issue
Block a user