grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

right-click works on a nameplate, and the honor tab names your rank

Browse Source 
- right-clicking a nameplate acts again: both buttons read the press pick, as the reference picks once on the down edge and never re-picks on release (2230)
- the honor tab's Highest Rank and title name the rank: a rank title's team digit is the race, not the faction group, so a GM-flagged character keeps its rank (2227)
- a raid's party feed watches only its members' descriptors, and the empty 2D opaque pass is skipped: ~0.7 ms a frame in a crowd (2225)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Sam
2026-09-15 16:57:38 +07:00
co-authored by Claude Fable 5.1
parent 0cc11b66a0
commit 37e55168c2
16 changed files with 697 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-app/src/addon_harness/mod.rs
+4
View File
@@ -1680,6 +1680,10 @@ fn seat_a_session(script: &mut UiScript) {
// `realm .. " - " .. faction` at file scope, so a nil faction is 24 addons stopping on
// `attempt to concatenate local 'faction'`. Every playable race has a side.
faction_group: Some("Alliance".into()),
// `0x5efe00`'s team digit for Human (race 1) — the rank-title key's second `%d`.
// Seated beside the faction group because they are different questions
// (`ui_unit::race_pvp_team`), not because they agree here.
pvp_team: crate::ui_unit::race_pvp_team(1),
..Default::default()
}),
);
crates/benilla-app/src/lib.rs
+1
View File
@@ -109,6 +109,7 @@ mod shaders;
mod game_tip;
mod name_persist;
mod opaque2d;
/// Where "the client is going down" may be observed, and why that is `Last` and not `Update`
/// (decision 1528). Every system that persists state on the way out registers through it.
mod shutdown;
crates/benilla-app/src/opaque2d.rs
+128
View File
@@ -0,0 +1,128 @@
//! **The 2D opaque pass, skipped when it has nothing to draw** (decision 2225).
//!
//! bevy's `MainOpaquePass2dNode` opens a command encoder and a render pass whether or not its two
//! phases hold an item, and on every camera of ours they never do: each player-UI quad is in the
//! transparent phase (`AlphaMode2d::Blend`, decision 0254), Bevy UI paints through its own node,
//! egui through its own. 2197 parked the pass as "price it on an immediate-mode GPU"; 2205 priced
//! its GPU at zero; the crowd profile of 2225 priced its CPU at ~0.2 ms a frame, parked, traced —
//! an encoder and a pass that wgpu sizes and clears its usage trackers for, empty or not (the
//! residency term `perf::gpu`'s census names).
//!
//! The pass exists in bevy for the clear, and the clear does not need it: whichever pass touches
//! the view target first clears it (`ColorAttachment`'s first-call semantics, the same rule
//! `static_gx` and the FFX combine already lean on), and bevy's transparent 2D node opens its pass
//! unconditionally for exactly that reason. So an empty opaque pass has nothing to do that the
//! pass after it does not.
//!
//! This is bevy's node with one early return, registered under bevy's own label:
//! `RenderGraph::add_node` is a map insert, so the later registration replaces the node and the
//! graph's edges — keyed by label — carry over untouched.
use bevy::core_pipeline::core_2d::graph::{Core2d, Node2d};
use bevy::core_pipeline::core_2d::{AlphaMask2d, Opaque2d};
use bevy::ecs::query::QueryItem;
use bevy::prelude::*;
use bevy::render::camera::ExtractedCamera;
use bevy::render::diagnostic::RecordDiagnostics;
use bevy::render::render_graph::{
NodeRunError, RenderGraphContext, RenderGraphExt, ViewNode, ViewNodeRunner,
};
use bevy::render::render_phase::{TrackedRenderPass, ViewBinnedRenderPhases};
use bevy::render::render_resource::{CommandEncoderDescriptor, RenderPassDescriptor, StoreOp};
use bevy::render::renderer::RenderContext;
use bevy::render::view::{ExtractedView, ViewDepthTexture, ViewTarget};
use bevy::render::RenderApp;
#[derive(Default)]
struct SkipEmptyOpaque2dNode;
impl ViewNode for SkipEmptyOpaque2dNode {
type ViewQuery = (
&'static ExtractedCamera,
&'static ExtractedView,
&'static ViewTarget,
&'static ViewDepthTexture,
);
fn run<'w>(
&self,
graph: &mut RenderGraphContext,
render_context: &mut RenderContext<'w>,
(camera, view, target, depth): QueryItem<'w, '_, Self::ViewQuery>,
world: &'w World,
) -> Result<(), NodeRunError> {
let (Some(opaque_phases), Some(alpha_mask_phases)) = (
world.get_resource::<ViewBinnedRenderPhases<Opaque2d>>(),
world.get_resource::<ViewBinnedRenderPhases<AlphaMask2d>>(),
) else {
return Ok(());
};
let view_entity = graph.view_entity();
let (Some(opaque_phase), Some(alpha_mask_phase)) = (
opaque_phases.get(&view.retained_view_entity),
alpha_mask_phases.get(&view.retained_view_entity),
) else {
return Ok(());
};
// The one line bevy's node does not have. Nothing to draw means nothing to encode: the
// transparent pass that follows opens its own pass unconditionally and takes the target's
// first-call clear.
if opaque_phase.is_empty() && alpha_mask_phase.is_empty() {
return Ok(());
}
let diagnostics = render_context.diagnostic_recorder();
let color_attachments = [Some(target.get_color_attachment())];
let depth_stencil_attachment = Some(depth.get_attachment(StoreOp::Store));
render_context.add_command_buffer_generation_task(move |render_device| {
let mut command_encoder =
render_device.create_command_encoder(&CommandEncoderDescriptor {
label: Some("main_opaque_pass_2d_command_encoder"),
});
let render_pass = command_encoder.begin_render_pass(&RenderPassDescriptor {
label: Some("main_opaque_pass_2d"),
color_attachments: &color_attachments,
depth_stencil_attachment,
timestamp_writes: None,
occlusion_query_set: None,
});
let mut render_pass = TrackedRenderPass::new(&render_device, render_pass);
let pass_span = diagnostics.pass_span(&mut render_pass, "main_opaque_pass_2d");
if let Some(viewport) = camera.viewport.as_ref() {
render_pass.set_camera_viewport(viewport);
}
if !opaque_phase.is_empty() {
if let Err(err) = opaque_phase.render(&mut render_pass, world, view_entity) {
error!("Error encountered while rendering the 2d opaque phase {err:?}");
}
}
if !alpha_mask_phase.is_empty() {
if let Err(err) = alpha_mask_phase.render(&mut render_pass, world, view_entity) {
error!("Error encountered while rendering the 2d alpha mask phase {err:?}");
}
}
pass_span.end(&mut render_pass);
drop(render_pass);
command_encoder.finish()
});
Ok(())
}
}
/// Replaces bevy's 2D opaque node under its own label. Added by [`crate::ui_pass::PlayerUiPlugin`],
/// which owns every camera that runs the `Core2d` graph, after `DefaultPlugins` has registered the
/// node it replaces.
pub(crate) struct SkipEmptyOpaque2dPlugin;
impl Plugin for SkipEmptyOpaque2dPlugin {
fn build(&self, app: &mut App) {
let Some(render_app) = app.get_sub_app_mut(RenderApp) else {
return;
};
render_app.add_render_graph_node::<ViewNodeRunner<SkipEmptyOpaque2dNode>>(
Core2d,
Node2d::MainOpaquePass,
);
}
}
crates/benilla-app/src/target/click.rs
+118 -16
View File
@@ -30,8 +30,10 @@ use super::*;
/// app's cursor-transition watcher (`crate::sound`), matching the ref's `ClearCursor` play.
pub(super) fn world_right_click_payload(
mut right_clicks: MessageReader<WorldRightClick>,
hovered: Res<Hovered>,
hovered_object: Res<HoveredObject>,
// The **press** pick, not the live hover (decision 2230) — this leg and [`act_on_right_click`]
// are two halves of one click's routing, so they must classify the same pick or a right-click
// can dismiss the cursor payload *and* interact with the thing it was held over.
press: Res<PressPick>,
script: Option<NonSendMut<UiScript>>,
) {
if right_clicks.read().last().is_none() {
@@ -40,7 +42,7 @@ pub(super) fn world_right_click_payload(
let Some(mut script) = script else {
return;
};
if hovered.target.is_some() || hovered_object.target.is_some() {
if press.hovered.target.is_some() || press.object.target.is_some() {
return;
}
script.clear_cursor_payload();
@@ -56,16 +58,21 @@ pub(super) fn world_right_click_payload(
/// replayed a [`WorldClick`] and was guarded on the press pick naming the same unit; the live probe
/// caught it — the scripted click selected nothing.
///
/// The RIGHT button is the other way round, deliberately. [`act_on_right_click`] acts on the LIVE
/// hover across many legs (interact, attack, loot, GameObject), so feeding it a unit the pointer is
/// not on could attack the wrong thing; it is replayed only when the hover already names the
/// plate's unit — the physical case. A scripted right-click on a plate does nothing yet, which is
/// a stated gap rather than a guess about which leg it should take.
/// The RIGHT button is the other way round, deliberately. [`act_on_right_click`] acts on the
/// frame's [`PressPick`] across many legs (interact, attack, loot, GameObject), so feeding it a
/// unit that pick does not name could attack the wrong thing; it is replayed only when the press
/// pick already names the plate's unit. A scripted right-click on a plate does nothing yet, which
/// is a stated gap rather than a guess about which leg it should take.
///
/// The **physical** right-click on a plate does not come through here at all and never did: the
/// press engages freelook (the camera looks through a plate — 2159), freelook hands the plates'
/// mouse back (`0x60f830`), and the release therefore hit-tests off the plate, so its `OnClick`
/// never fires. That gesture is the camera arbiter's own [`WorldRightClick`], acting on the pick
/// the press latched — which is exactly what decision 2230 made it read.
#[allow(clippy::too_many_arguments)]
pub(super) fn select_on_plate_click(
mut plate: ResMut<crate::vplates::PlateClicks>,
press: Res<PressPick>,
hovered: Res<Hovered>,
ground: Res<crate::ui_action::SpellTargeting>,
mut selection: ResMut<Selection>,
mut seam: crate::creature_anim::AttackSeam,
@@ -95,7 +102,7 @@ pub(super) fn select_on_plate_click(
greeting.write(crate::sound::NpcGreetingRequest { npc: entity });
// The attack classification is the press pick's, and only when the press was actually on
// this unit: a scripted click has no cursor behind it, so it is not an attack-cursor click.
let attack = press.attack && press.hovered.target == Some(entity);
let attack = press.attack() && press.hovered.target == Some(entity);
scan::commit(
&mut selection,
&mut seam,
@@ -107,7 +114,7 @@ pub(super) fn select_on_plate_click(
attack,
);
}
if right.into_iter().any(|e| hovered.target == Some(e)) {
if right.into_iter().any(|e| press.hovered.target == Some(e)) {
right_clicks.write(WorldRightClick);
}
}
@@ -179,7 +186,7 @@ pub(super) fn select_on_click(
stores.get(entity).ok(),
engaged,
self_guid,
press.attack,
press.attack(),
);
}
// Clicked nothing targetable → deselect (only sends the clear if we actually had a
@@ -288,9 +295,16 @@ fn interaction_already_open_on(target: u64, interact: &crate::ui_session::Intera
#[allow(clippy::too_many_arguments, clippy::type_complexity)]
pub(super) fn act_on_right_click(
mut clicks: MessageReader<WorldRightClick>,
hovered: Res<Hovered>,
hovered_object: Res<HoveredObject>,
cursor: Res<WorldCursor>,
// **The press pick, not the live hover** (decision 2230) — the same latch the left button has
// read since 1122, and for the same reason: the reference picks exactly once, on the down edge
// (`0x481f00`, one caller image-wide), into the WorldFrame's own slots, and nothing re-picks on
// move or on release. Reading the live hover here only looked right because the release frame
// re-picks at the restored cursor and, for a body click that moved nothing, lands back on the
// same unit. A V-plate has no body under the cursor to land back on: the plate published the
// mouseover at the press (`0x7cb850` → `0x492890`), freelook then took the plates' mouse away
// (`0x60f830`), and the live hover at the release was empty — so a right-click on a plate
// reached every branch below with nothing hovered and did nothing at all.
press: Res<PressPick>,
mut selection: ResMut<Selection>,
mut seam: crate::creature_anim::AttackSeam,
self_player: Query<(Entity, &Guid, Has<Engaged>), With<SelfPlayer>>,
@@ -333,6 +347,8 @@ pub(super) fn act_on_right_click(
if clicks.read().last().is_none() {
return;
}
// The whole ladder below reads ONE pick — the gesture's own, frozen at the press.
let (hovered, hovered_object, cursor) = (&press.hovered, &press.object, &press.cursor);
// ── The interact family's ACTOR gate: am I in the saddle? ────────────────────────────────
// One predicate, the reference's own — the PLAYER's `UNIT_FIELD_MOUNTDISPLAYID` (decision
// 0481's "one mounted predicate"; wow-re `mounted-action-gate.md`: no aura, no taxi
@@ -359,7 +375,7 @@ pub(super) fn act_on_right_click(
// suppresses the send with no toast (the reference auto-walks there instead — `0x610300`, also
// no packet). The lock arm runs first in `0x5f3130`, and it does here too: a `Refuse` toasts
// even when we are also out of range.
if go_is_nearest(&hovered, &hovered_object) {
if go_is_nearest(hovered, hovered_object) {
// **The interact chain's first link** (tag `use`). "I clicked it and nothing happened" spans
// three systems — this decision, the `CMSG_GAMEOBJ_USE` it sends, and the `SMSG_SPELL_GO`
// the server answers with — and until this line existed only the *taken* branches said
@@ -2246,4 +2262,90 @@ mod tests {
set(&mut world, None);
assert_eq!(run(&mut world, "TargetLastEnemy()"), None);
}
use crate::net::{ClientCommand, NetCommands, ObjectStore};
use bevy::ecs::system::RunSystemOnce;
const F_HEALTH: u16 = 22;
const F_MAXHEALTH: u16 = 28;
const BOAR: u64 = 0xB0A2;
const ME: u64 = 0x5E1F;
fn store(pairs: &[(u16, u32)]) -> ObjectStore {
ObjectStore(benilla_protocol::ObjectFields::from_pairs(pairs))
}
/// Everything [`act_on_right_click`] and the commit under it reach for, and nothing else.
fn right_click_world() -> (World, Entity) {
let (tx, _rx) = crossbeam_channel::unbounded::<ClientCommand>();
let mut world = World::new();
world.insert_resource(NetCommands(tx));
world.init_resource::<Messages<WorldRightClick>>();
world.init_resource::<PressPick>();
world.init_resource::<Selection>();
world.init_resource::<crate::ui_cast::QueuedMeleeSpell>();
world.init_resource::<crate::ui_action::AutoRepeatActive>();
world.init_resource::<Messages<crate::creature_anim::SheathRequest>>();
world.init_resource::<Messages<crate::player::StandStateRequest>>();
world.init_resource::<crate::creature_anim::GestureQueue>();
world.init_resource::<crate::go_templates::GameObjectTemplates>();
world.init_resource::<crate::items::Items>();
world.init_resource::<crate::ui_action::PlayerActions>();
world.init_resource::<crate::ui_action::LearnedAbilities>();
world.init_resource::<crate::ui_quest::QuestGiver>();
world.init_resource::<crate::ui_binder::BinderState>();
world.init_resource::<crate::death::DeathNet>();
world.init_resource::<crate::ui_session::InteractNpc>();
world.init_resource::<crate::ui_action::UiErrorKeys>();
world.init_resource::<crate::ui_action::CastErrors>();
world.init_resource::<crate::ui_loot::LootLatch>();
world.init_resource::<crate::ui_mail::MailOpen>();
world.init_resource::<crate::ui_item_text::ItemTextOpen>();
world.init_resource::<crate::ui_action::GoOpenerCasts>();
world.spawn((SelfPlayer, Guid(ME)));
let boar = world
.spawn((Guid(BOAR), store(&[(F_HEALTH, 100), (F_MAXHEALTH, 100)])))
.id();
(world, boar)
}
/// **A right-click that began on a V-plate acts on that plate's unit** — the bug the director
/// reported as "right click on nameplates no longer works", and the reason
/// [`act_on_right_click`] reads [`PressPick`] (decision 2230).
///
/// The gesture it reconstructs is the whole of it: the plate published the mouseover at the
/// press, the press engaged freelook (the camera looks *through* a plate — 2159), freelook
/// handed the plates' mouse back (`0x60f830`) so the plate's own `OnClick` never fires and its
/// hover goes dark, and `update_hover` blanks the live [`Hovered`] for the whole session. What
/// survives all of that is the press latch — and a plate hover leaves **no body under the
/// cursor** for the release frame's re-pick to land back on, which is exactly why the live
/// hover carried the body case and dropped this one.
#[test]
fn a_right_click_begun_on_a_plate_acts_on_the_plates_unit() {
let (mut world, boar) = right_click_world();
// The press: the plate's unit is the mouseover (distance 0.0 — topmost UI), classified
// Attack. Nothing here is live state; it is the latch, exactly as `latch_press_pick` wrote
// it one frame before the look session began.
*world.resource_mut::<PressPick>() = PressPick {
hovered: Hovered {
target: Some(boar),
guid: Some(BOAR),
distance: 0.0,
..Hovered::default()
},
cursor: WorldCursor {
kind: cursor_mode::CursorKind::Attack,
unable: false,
},
..PressPick::default()
};
world
.resource_mut::<Messages<WorldRightClick>>()
.write(WorldRightClick);
world.run_system_once(act_on_right_click).unwrap();
assert_eq!(
world.resource::<Selection>().guid,
Some(BOAR),
"the release must act on the unit whose plate the press was over"
);
}
}
crates/benilla-app/src/target/mod.rs
+21 -5
View File
@@ -242,14 +242,30 @@ impl Default for PickOcclusion {
///
/// Written by [`latch_press_pick`] at the head of the target chain, which runs **before**
/// [`hover::update_hover`] can clear anything, so the values it copies are the pick as of the press.
///
/// **Both buttons read it** (decision 2230). 1122 built this for the left button and left
/// [`click::act_on_right_click`] on the live hover, where it appeared to work only because the
/// release frame re-picks at the restored cursor and — for a body click that moved nothing —
/// lands back on the same unit. It is not the same pick: the plate case has no body under the
/// cursor to re-find, so a right-click on a V-plate acted on nothing at all.
#[derive(Resource, Default, Clone, Copy)]
pub(crate) struct PressPick {
pub(crate) hovered: Hovered,
pub(crate) object: HoveredObject,
pub(crate) occlusion: PickOcclusion,
/// The cursor's Attack classification at the press — the reference's new-target validation
/// (`0x5ecb70`), which likewise reads the pick the gesture started on, not a live one.
pub(crate) attack: bool,
/// The **context cursor as of the press** — the classification the whole right-click ladder
/// forks on (Attack/Speak/Loot/the GameObject arms) plus its `unable` range gray, and the
/// reference's new-target validation (`0x5ecb70`) likewise reads the pick the gesture started
/// on, not a live one. Latched whole rather than as the single Attack bit it used to be,
/// because every one of those forks is the same frozen pick's answer.
pub(crate) cursor: cursor_mode::WorldCursor,
}
impl PressPick {
/// Was the press an **Attack**-cursor press? `0x5ecb70`'s new-target validation.
pub(crate) fn attack(&self) -> bool {
self.cursor.kind == cursor_mode::CursorKind::Attack
}
}
/// Latch the frame's pick as a press begins — the [`PressPick`] writer.
@@ -281,7 +297,7 @@ pub(crate) fn latch_press_pick(
hovered: *hovered,
object: *object,
occlusion: *occlusion,
attack: cursor.kind == cursor_mode::CursorKind::Attack,
cursor: *cursor,
};
}
@@ -567,7 +583,7 @@ mod tests {
.press(MouseButton::Left);
world.run_system(id).unwrap();
assert_eq!(world.resource::<PressPick>().hovered.guid, Some(BOAR));
assert!(world.resource::<PressPick>().attack, "Attack rode along");
assert!(world.resource::<PressPick>().attack(), "Attack rode along");
// The drag begins: the look session blanks the live hover, every frame, for as long as it
// lasts. The latch must not follow it down.
crates/benilla-app/src/ui_battlefield_score.rs
+6 -7
View File
@@ -78,13 +78,12 @@ fn resolve_board(
let (race, class) = names
.player_traits(r.guid)
.map_or((0, 0), |(race, class, _)| (race, class));
// `0` Horde, `1` Alliance, `-1` neither — the reference's mask walk lands on the same
// three values (§6.2), and every playable race has a fixed side in 1.12.
let faction = match crate::ui_unit::race_faction_group(race) {
Some("Alliance") => 1,
Some(_) => 0,
None => -1,
};
// `0` Horde, `1` Alliance, `-1` neither. `0x4aa200` walks the name-cache record's race
// through `ChrRaces` → `FactionTemplate` → factionGroupMask, which is `0x5efe00`'s walk
// with the null-row guards omitted — so it is [`crate::ui_unit::race_pvp_team`] and not a
// second copy of it. (It used to be one, re-derived here out of `race_faction_group`; a
// second copy of this walk is exactly what report B378 was, one surface over.)
let faction = i32::from(crate::ui_unit::race_pvp_team(race));
let mut stats = [0u32; 8];
for (slot, v) in stats.iter_mut().zip(&r.stats) {
*slot = *v;
crates/benilla-app/src/ui_honor.rs
+13 -5
View File
@@ -96,11 +96,19 @@ impl Plugin for UiHonorPlugin {
/// Read the honor block off our own descriptor, or `None` while none of it has streamed.
///
/// **`None` and all-zeroes are different states and the difference is visible**: a fresh character
/// who has never fought has every counter at 0 *and* the fields present, while a player who has
/// only just entered the world may have none of them yet. The reference paints the second as
/// blank, not as zero, so the feed pushes nothing until at least one field arrives rather than
/// inventing a zeroed snapshot.
/// **`None` and all-zeroes are the same thing on screen, and that is the point of the gate, not a
/// flaw in it.** This doc used to say the reference "paints a player whose fields have not arrived
/// as blank, not as zero". It does not: `0x51a4b0`–`0x51a7c0`'s "absent → 0.0" tails are about the
/// absent player OBJECT, and once the object exists every one of those bindings reads a
/// descriptor array that is allocated and zeroed, so a field the server never sent reads `0`
/// (wow-re `honor-panel-law.md` §3.1–3.5). Our bindings answer the same zeros —
/// `honor(lua).unwrap_or_default()` — so what the gate actually decides is only *when the first
/// snapshot is pushed*, and with it when `PLAYER_PVP_KILLS_CHANGED`/`PLAYER_PVP_RANK_CHANGED`
/// first fire. It cannot make a row differ from the reference.
///
/// That is worth stating because this gate was the other candidate cause of report B378, and it
/// is ruled out by exactly this: a missing snapshot and a zeroed one paint the same pane. The
/// cause was the rank title's team digit (decision 2227).
fn honor_snapshot(store: &ObjectStore) -> Option<HonorState> {
let f = &store.0;
let session = f.player_session_kills();
crates/benilla-app/src/ui_party/feed.rs
+20 -7
View File
@@ -114,7 +114,7 @@ pub(super) fn feed_party(
stores: Query<&ObjectStore>,
changed_stores: Query<(), Changed<ObjectStore>>,
mut removed_stores: RemovedComponents<ObjectStore>,
self_q: Query<(&Guid, &ObjectStore), With<SelfPlayer>>,
self_q: Query<(Entity, &Guid, &ObjectStore), With<SelfPlayer>>,
factions: Option<Res<crate::target::Factions>>,
names: Res<NameCache>,
areas: Option<Res<crate::area::AreaTableRes>>,
@@ -152,7 +152,20 @@ pub(super) fn feed_party(
let area_moved = fed.area.moved(here.area().map_or(u64::MAX, u64::from));
let group_changed = group.is_changed();
let index_changed = index.is_changed();
let stores_changed = !changed_stores.is_empty();
// The members' descriptors and our own — the only stores the merged view below reads. This
// used to be "any store in the world moved", which in a crowd is true on every frame: the
// crowd profile of 2225 read this feed re-pushing a 39-member raid to the VM every frame of
// the pin, 0.47 ms traced, for rows that had not changed.
let stores_changed = self_q
.iter()
.next()
.is_some_and(|(e, _, _)| changed_stores.get(e).is_ok())
|| group.members.iter().any(|m| {
index
.0
.get(&m.guid)
.is_some_and(|&e| changed_stores.get(e).is_ok())
});
let stores_removed = !removed_stores.is_empty();
let factions_changed = factions.as_ref().is_some_and(|r| r.is_changed());
let areas_changed = areas.as_ref().is_some_and(|r| r.is_changed());
@@ -189,12 +202,12 @@ pub(super) fn feed_party(
return;
}
let self_pair = self_q.iter().next();
let self_guid = self_pair.map(|(g, _)| g.0);
let self_guid = self_pair.map(|(_, g, _)| g.0);
// The party's PvP faction group (decision 0646 §1): our own. A 1.12 party is always one
// faction, and a member out of streaming range has no descriptor to resolve one from — so
// reading it off ourselves is exact for every member, present or not.
let own_group =
self_pair.and_then(|(_, store)| crate::ui_unit::faction_group(store, factions.as_deref()));
let own_group = self_pair
.and_then(|(_, _, store)| crate::ui_unit::faction_group(store, factions.as_deref()));
// The party1..4 slot view: own-subgroup members, packet order (`GroupState::party_slots`,
// the 0440 byte law) — in a plain party this is simply the roster.
@@ -257,7 +270,7 @@ pub(super) fn feed_party(
};
// The raid roster — `GetNumRaidMembers`/`GetRaidRosterInfo`/`UnitInRaid` all read this one
// list, so the count can never disagree with the array the way it would if we kept both.
let me = self_pair.map(|(g, store)| RaidSelf {
let me = self_pair.map(|(_, g, store)| RaidSelf {
guid: g.0,
flags: group.own_flags,
level: store.0.unit_level().unwrap_or(0),
@@ -372,7 +385,7 @@ pub(super) fn feed_party(
for (i, token) in RAID_TOKENS.iter().enumerate() {
let snap = raid_guids.get(i).and_then(|guid| {
if Some(*guid) == self_guid {
let (_, store) = self_pair?;
let (_, _, store) = self_pair?;
let name = names.peek(*guid).map(str::to_string);
let mut s = crate::ui_unit::snapshot(store, name, 0, chr);
s.is_player = true;
crates/benilla-app/src/ui_pass.rs
+2
View File
@@ -578,6 +578,8 @@ impl Plugin for PlayerUiPlugin {
Material2dPlugin::<UiQuadMaterial>::default(),
// Owned here, not in main.rs: the lane's decode is not optional (see its doc).
crate::ui_gamma::UiGammaPlugin,
// The 2D opaque pass skipped when empty — which on this camera is always (2225).
crate::opaque2d::SkipEmptyOpaque2dPlugin,
))
.add_systems(Startup, (spawn_ui_camera, init_white_texture))
.configure_sets(
crates/benilla-app/src/ui_script/honor_frame_tests.rs
+70 -3
View File
@@ -96,6 +96,11 @@ fn alliance_player() -> UnitState {
level: 60,
sex: 2,
faction_group: Some("Alliance".into()),
// The rank title's team digit is `pvp_team` (`0x5efe00`: race → ChrRaces →
// FactionTemplate), NOT `faction_group` (`UnitFactionGroup`: the live template). Human is
// race 1 → Alliance → 1. Both are seated because a real Alliance player has both; the GM
// test below is what drives them apart.
pvp_team: crate::ui_unit::race_pvp_team(1),
race: Some("Human".into()),
race_file: Some("Human".into()),
class: Some("Warrior".into()),
@@ -206,6 +211,68 @@ fn an_unranked_character_reads_none_and_shows_no_badge() {
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
}
/// **Report B378 — a GM-flagged Grand Marshal's Highest Rank read `None`.** The regression test
/// for decision 2227, and the one that holds the two sides apart.
///
/// vmangos forces a GM to faction template 35, whose group mask is 0, so `UnitFactionGroup`
/// genuinely answers nil — that half is faithful and the PvP icon is *supposed* to vanish with it.
/// The rank title is not on that wire at all: `0x5efe00` reads the unit's RACE through
/// `ChrRaces`/`FactionTemplate` (`[obj+0x110]+0x78`), and a GM's race does not change, which is why
/// the reporter's own 1.12 client on the same server read "Grand Marshal" from the same byte. We
/// had the title keyed off the faction group, so both rows went to NONE together.
///
/// The control is the pair: the two rank rows must name the rank **and** the bar must still take
/// the sideless arm's Horde red — a fix that "restored the side" would pass the first half and
/// silently break the second. Skips without client data.
#[test]
fn a_gm_flagged_player_keeps_his_rank_title_and_loses_only_the_faction_group() {
let _data = benilla_formats::wow_data_or_skip!();
let mut s = shown_honor_page();
s.set_honor(Some(HonorState {
// Grand Marshal — internal 18, the top of the Alliance list.
rank: 18,
highest_rank: 18,
..state()
}));
s.set_unit(
"player",
Some(UnitState {
// `.gm on`: template 35, group mask 0 → no side.
faction_group: None,
faction_group_localized: None,
pvp_rank: 18,
..alliance_player()
}),
);
s.run("HonorFrame_Update(1)").unwrap();
s.resolve();
assert_eq!(
text(&mut s, "HonorFrameLifeTimeRankValue"),
"Grand Marshal",
"the Highest Rank row — the reported symptom"
);
assert_eq!(
text(&mut s, "HonorFrameCurrentPVPTitle"),
"Grand Marshal",
"and the title under the badge, which reads the same key"
);
assert!(
s.eval::<bool>(r#"return UnitFactionGroup("player") == nil"#)
.unwrap(),
"the side itself is still gone — that half of GM mode is the reference's"
);
// The control: the bar's colour is `UnitFactionGroup`'s, so a sideless player takes the
// `else` arm (Horde red, `HonorFrame.lua:68`) and must KEEP taking it.
let (r, g, b) = s
.eval::<(f64, f64, f64)>("return HonorFrameProgressBar:GetStatusBarColor()")
.unwrap();
assert!(
r > g && r > b,
"a sideless player keeps the else-arm's red bar, got ({r}, {g}, {b})"
);
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
}
/// The bar takes the 0..1 fraction straight, and wears the player's own faction colour. Alliance
/// navy here; the `else` arm is Horde red for everyone else, including a unit whose side has not
/// resolved — a bar with no colour at all would read as a broken pane.
@@ -298,11 +365,11 @@ fn the_real_global_strings_name_the_rank() {
load_page(&s);
s.set_honor(Some(state()));
for (group, want) in [("Alliance", "Knight-Captain"), ("Horde", "Legionnaire")] {
for (team, want) in [(1i8, "Knight-Captain"), (0, "Legionnaire")] {
s.set_unit(
"player",
Some(UnitState {
faction_group: Some(group.into()),
pvp_team: team,
..alliance_player()
}),
);
@@ -312,7 +379,7 @@ fn the_real_global_strings_name_the_rank() {
assert_eq!(
text(&mut s, "HonorFrameCurrentPVPTitle"),
want,
"rank 12 to the {group}"
"rank 12 to team {team}"
);
}
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
crates/benilla-app/src/ui_unit.rs
+137 -9
View File
@@ -605,6 +605,42 @@ pub(crate) fn race_faction_group(race: u8) -> Option<&'static str> {
})
}
/// A unit's **PvP team digit** — `0x5efe00`'s tri-state: `0` Horde, `1` Alliance, `-1` no side.
///
/// **This is NOT [`faction_group`], and the difference is the whole of report B378.** The two read
/// different sources and only agree while nothing has moved a unit off its racial faction:
///
/// * `UnitFactionGroup` (`0x516630`) reads the unit's LIVE `UNIT_FIELD_FACTIONTEMPLATE`
/// (`0x5166b8 mov eax,[eax+0x110]` / `0x5166be mov eax,[eax+0x74]`, byte-read here) — so a
/// vmangos GM, forced to template 35, genuinely has no side and the PvP flag icon genuinely
/// hides. That is faithful.
/// * The rank title's team digit (`0x5efe00`) reads the unit's **RACE** and walks
/// `[obj+0x110]+0x78` → `ChrRaces.dbc` field 2 (FactionTemplate id) → `FactionTemplate.dbc`
/// field 3 (factionGroupMask) → `& 4` ⇒ 0, else `& 2` ⇒ 1, else −1 — never the live template.
/// A GM's race does not change, so the reference names his rank exactly as it always did.
///
/// We had the second wired to the first, which is why a Grand Marshal's Honor tab read `NONE` on
/// a GM-flagged account while the 1.12 client on the same server read "Grand Marshal"
/// (decision 2227). Every `0x5efe00` caller is race-derived: `GetPVPRankInfo`'s team
/// (`0x51a9af`/`0x51a9c8`), `UnitPVPName`'s rank decoration (`0x5efe60`), the battlefield
/// scoreboard's per-row side (`0x4aa200`, which inlines the same walk off the name-cache record).
///
/// The table is the shipped one, frozen: `ChrRaces.dbc` has **nine** rows in 5875 and race 9
/// (Goblin, unplayable) shares Human's faction template 1, so it answers Alliance — not `None`,
/// which is what a "playable races only" table would say.
/// [`tests::race_pvp_team_matches_the_shipped_tables`] walks the real DBCs and pins every row.
pub(crate) fn race_pvp_team(race: u8) -> i8 {
match race {
// factionGroupMask 3 = Player|Alliance → `& 4` clear, `& 2` set.
1 | 3 | 4 | 7 | 9 => 1,
// factionGroupMask 5 = Player|Horde → `& 4` set, tested first.
2 | 5 | 6 | 8 => 0,
// No `ChrRaces` row (a creature's race byte, or an unstreamed descriptor): the engine's
// bounds/NULL failure tail, `-1`. It formats into the key and matches no GlobalString.
_ => -1,
}
}
/// Resolve a UnitPopup unit token to the **player guid** it names — `"target"` through the
/// selection iff it really is a player (the target frame's PLAYER menu), a `"partyN"` token through
/// the roster (the party frame's PARTY menu). `"player"` (yourself) and any unresolved token answer
@@ -1004,6 +1040,13 @@ pub(crate) fn snapshot(
// pane's `UnitPVPRank("target")` can answer at all. A creature has no PLAYER block and
// reads 0, the reference's own answer for one.
pvp_rank: store.0.player_pvp_rank().unwrap_or(0),
// `0x5efe00`'s team digit — the second `%d` of `PVP_RANK_<rank>_<team>`. It sits here
// beside the rank byte for the same reason that one does (nothing but the descriptor is
// needed) and it reads the RACE, not `faction_group`: the engine walks the race through
// `ChrRaces`/`FactionTemplate` and never looks at the live `UNIT_FIELD_FACTIONTEMPLATE`
// this unit is carrying, so a GM-flagged player keeps his rank title while losing the PvP
// icon. See [`race_pvp_team`] and decision 2227 (report B378).
pvp_team: store.0.unit_race().map_or(-1, race_pvp_team),
// `PLAYER_BYTES_3` byte 2 — the city-protector title, the same PUBLIC dword as the rank
// byte above. `UnitPVPName` appends a `PVP_MEDAL<n>` line for a non-zero one; 0 is "no
// medal", which is every character on this server (vmangos never writes the byte).
@@ -1556,20 +1599,22 @@ fn feed_units(
// being broken, which has now cost two separate sessions an investigation. So it is a BENCH
// diagnostic, not UI: nothing appears on screen, exactly as in the reference.
//
// The honor arc (1512) put a SECOND surface behind this same side: a rank's title is the
// GlobalString `PVP_RANK_<rank>_<team>`, and with no side there is no team digit, so
// `GetPVPRankInfo` answers nil and the Honor tab renders `NONE` at every rank — for a Grand
// Marshal. That reads exactly like an unbuilt pane, which is why it is named in the warning
// rather than left for the next investigation to rediscover.
// The honor arc (1512) was once listed here as a SECOND surface behind this same side, and
// it is not one: the rank title's team digit is `0x5efe00`, which reads the RACE through
// `ChrRaces`/`FactionTemplate` and never the live template, so a GM's Honor tab names his
// rank exactly as it always did (`race_pvp_team`, decision 2227 — the cause of report B378
// was that we had wired the two together, not the GM mode itself). What this warning still
// covers is every genuinely template-derived surface: `UnitFactionGroup` and the icons and
// comparisons built on it.
if let Some(p) = &player {
let sideless = p.faction_group.is_none();
if sideless && !feed.warned_sideless {
warn!(
"faction: our own template names no side (usually GM mode — vmangos forces \
template 35, group mask 0). Faction-derived UI cannot resolve a side while this \
holds: the PvP flag icon stays hidden however flagged you are, and the Honor tab's rank \
title reads NONE at every rank (PVP_RANK_<rank>_<team> has no team digit). \
`.gm off` restores both."
template 35, group mask 0). Every UnitFactionGroup-derived surface loses its \
side while this holds — the PvP flag icon stays hidden however flagged you are. \
`.gm off` restores it. (The Honor tab's rank title is NOT one of these: its team \
digit comes from your race, not your template.)"
);
}
feed.warned_sideless = sideless;
@@ -2158,6 +2203,89 @@ fn combo_edge(last: Option<(u8, u64)>, now: (u8, u64)) -> Option<bool> {
mod tests {
use super::*;
/// The DESCRIPTOR leg of the same answer: `snapshot` takes the team digit off
/// `UNIT_FIELD_BYTES_0` byte 0 and **never** off `UNIT_FIELD_FACTIONTEMPLATE`.
///
/// The pane-level regression (`ui_script::honor_frame_tests::a_gm_flagged_player_…`) seats
/// `pvp_team` by hand, so it proves the key is built from the right field and not that the
/// right field is read. This is that half: a template-35 GM — the exact descriptor vmangos
/// gives one — still answers his race's side.
#[test]
fn the_team_digit_comes_off_the_race_byte_not_the_faction_template() {
use benilla_protocol::ObjectFields;
/// `UNIT_FIELD_FACTIONTEMPLATE` / `UNIT_FIELD_BYTES_0`, one dword apart — the reference's
/// own `[obj+0x110]+0x74` and `+0x78`.
const FACTIONTEMPLATE: u16 = 35;
const BYTES_0: u16 = 36;
/// vmangos's GM template: `FactionTemplate.dbc` group mask 0, friendly to everyone.
const GM_TEMPLATE: u32 = 35;
let team = |fields: &[(u16, u32)]| {
snapshot(
&ObjectStore(ObjectFields::from_pairs(fields)),
None,
0,
None,
)
.pvp_team
};
// Byte 0 of BYTES_0 is the race; the class in byte 1 must not disturb it.
let human_warrior = 1 | (1 << 8);
let scourge_mage = 5 | (8 << 8);
assert_eq!(team(&[(BYTES_0, human_warrior)]), 1, "Human → Alliance");
assert_eq!(team(&[(BYTES_0, scourge_mage)]), 0, "Scourge → Horde");
// **The report.** The sideless GM template sits right beside the race byte and is not
// consulted: the answer is the race's, unchanged.
assert_eq!(
team(&[(BYTES_0, human_warrior), (FACTIONTEMPLATE, GM_TEMPLATE)]),
1,
"a GM keeps his race's side (report B378)"
);
assert_eq!(
team(&[(BYTES_0, scourge_mage), (FACTIONTEMPLATE, GM_TEMPLATE)]),
0,
"…on both sides"
);
// A unit whose race byte has not streamed is the engine's bounds-failure −1, and a
// faction template alone cannot stand in for it.
assert_eq!(team(&[]), -1, "no race byte, no team digit");
assert_eq!(
team(&[(FACTIONTEMPLATE, 1)]),
-1,
"and a template is not one"
);
}
/// [`race_pvp_team`]'s frozen table against the **shipped tables it is a copy of** — the walk
/// the engine runs at `0x5efe00`, on the real `ChrRaces.dbc` and `FactionTemplate.dbc`.
///
/// The table is hardcoded because it is nine constant rows of a 2006 file and threading a DBC
/// resource through every unit snapshot to read them would be pure ceremony. This is what
/// makes that safe: the file decides, and a row that ever disagrees — or a race the file has
/// and the table does not (race 9, Goblin, which shares Human's template and is **not** a
/// `None`) — fails here. Skips without client data.
#[test]
fn race_pvp_team_matches_the_shipped_tables() {
let data = benilla_formats::wow_data_or_skip!();
let mut chain = benilla_formats::open_chain(&data).expect("open chain");
let want = benilla_formats::load_race_pvp_teams(&mut chain).expect("ChrRaces walk");
// The misparse guard: 5875 ships nine rows, not eight. An empty or truncated map would
// otherwise let this test pass by asserting nothing.
assert_eq!(want.len(), 9, "ChrRaces.dbc row count");
for (&race, &team) in &want {
assert_eq!(race_pvp_team(race), team, "race {race}");
}
// Both sides are actually represented — a walk that answered one digit for everything
// would satisfy the loop above and name every rank off one list.
assert!(want.values().any(|&t| t == 0), "some race is Horde");
assert!(want.values().any(|&t| t == 1), "some race is Alliance");
// Off the end of the file is the engine's bounds-failure tail, not a guess.
for race in [0u8, 10, 255] {
assert!(!want.contains_key(&race));
assert_eq!(race_pvp_team(race), -1, "race {race} has no ChrRaces row");
}
}
/// **The tapped bit fires `UNIT_FACTION`, and without this the verbs are decorative.**
///
/// `UnitIsTapped` answering correctly is only half of it. pfUI's grey-bar branch lives inside
crates/benilla-formats/src/lib.rs
+3
View File
@@ -307,6 +307,9 @@ mod text_filter_lists;
pub use text_filter_lists::{load_chat_profanity, load_spam_messages, FilterPattern};
mod race_sound;
pub use race_sound::{load_exploration_sound_catalog, ExplorationSoundCatalog};
mod race_pvp_team;
pub use race_pvp_team::load_race_pvp_teams;
mod zone_map;
pub use zone_map::{load_zone_map, ZONE_MAP_EDGE};
crates/benilla-formats/src/race_pvp_team.rs
+76
View File
@@ -0,0 +1,76 @@
//! `ChrRaces.dbc` → the **PvP team digit** — `0x5efe00`, the second `%d` of
//! `PVP_RANK_<rank>_<team>`.
//!
//! The engine never asks a unit what faction template it is *currently* carrying for this; it asks
//! what RACE it is, and walks the race's own table:
//!
//! ```text
//! 5efe06 movzx eax, BYTE PTR [[obj+0x110]+0x78] ; UNIT_FIELD_BYTES_0 byte 0 = race
//! -> ChrRaces [0xc0dee0][race] +0x08 ; field 2 = FactionTemplate id
//! -> FactionTemplate [0xc0dd3c][id] +0x0c ; field 3 = factionGroupMask
//! 5efe42 test al,4 ; je ... ; mask & 4 -> 0 (Horde)
//! 5efe49 and al,2 ; … ; mask & 2 -> 1 (Alliance), else -1
//! 5efe54 (any bound/NULL failure) -> -1
//! ```
//!
//! (wow-re `system/ui/scratch/honor-panel-law.md` §3.11, VERIFIED; the `0`/`1` assignment is
//! settled there against the shipped `FactionGroup.dbc` and `GlobalStrings.lua`, not inferred.)
//!
//! **This is not `UnitFactionGroup`**, which reads the live `UNIT_FIELD_FACTIONTEMPLATE`
//! (`0x5166b8`/`0x5166be`) and so genuinely loses its side under GM mode. Confusing the two is
//! report B378 / decision 2227.
//!
//! Loaded only by [`crate::race_pvp_team::load_race_pvp_teams`]'s one consumer — the test that
//! pins `ui_unit::race_pvp_team`'s frozen table to the shipped tables. The runtime answer is that
//! table: nine constant rows of a 2006 file are not worth a DBC resource threaded through every
//! unit snapshot, but they are worth a test that fails if the file ever disagrees.
use std::collections::HashMap;
use anyhow::{Context, Result};
use benilla_dbc::{FieldType, Schema, SchemaField};
use crate::dbc::{parse, u32_at};
use crate::Chain;
const CHR_RACES: &str = "DBFilesClient\\ChrRaces.dbc";
/// Every `ChrRaces.dbc` row's team digit, by race id — `0` Horde, `1` Alliance, `-1` no side.
///
/// The map holds one entry per row the file actually has (nine in 5875, ids 1–9); a race id with
/// no row is the engine's `-1` tail and is simply absent here.
pub fn load_race_pvp_teams(chain: &mut Chain) -> Result<HashMap<u8, i8>> {
let bytes = chain
.read_file(CHR_RACES)
.with_context(|| format!("reading {CHR_RACES}"))?;
// 29 columns in 5875; the string columns are declared here only so the field count matches —
// this loader reads none of them. Same schema as `crate::characters`' creation catalog.
let mut schema = Schema::new("ChrRaces");
for i in 0..29 {
let ty = match i {
15 | 26 | 27 | 28 => FieldType::String,
_ => FieldType::UInt32,
};
schema.add_field(SchemaField::new(format!("f{i}"), ty));
}
let rs = parse(&bytes, schema, "ChrRaces")?;
let factions = crate::load_faction_catalog(chain)?;
let mut out = HashMap::new();
for r in rs.records() {
let (Some(race), Some(template)) = (u32_at(r, 0), u32_at(r, 2)) else {
continue;
};
let Ok(race) = u8::try_from(race) else {
continue;
};
// `0x5efe42`'s order: the Horde bit is tested FIRST, so a hypothetical row carrying both
// side bits would read Horde. Reproduced rather than tidied.
let team = match factions.template(template).map(|t| t.group_mask) {
Some(m) if m & 4 != 0 => 0,
Some(m) if m & 2 != 0 => 1,
_ => -1,
};
out.insert(race, team);
}
Ok(out)
}
crates/benilla-ui/src/script/pvp/mod.rs
+16 -16
View File
@@ -348,22 +348,22 @@ fn global_string(lua: &Lua, key: &str) -> Option<String> {
}
/// A unit's PvP **team digit** — `0x5efe00`'s tri-state: `0` Horde, `1` Alliance, `-1` for a unit
/// with no side (a neutral/monster faction template, or a race the tables can't resolve).
/// whose race resolves to no side.
///
/// The engine walks race → `ChrRaces` → `FactionTemplate` and tests `[rec+0xc]`'s factionGroupMask
/// `& 4` → 0, `& 2` → 1, else −1. This crate holds no DBC (decision 0068 §3), so the same answer
/// arrives pre-resolved on the snapshot as [`UnitState::faction_group`](super::UnitState) — the
/// app resolves it from `UNIT_FIELD_FACTIONTEMPLATE` through the same mask. Different hop, same
/// output space (the credit line's own inline copy at `0x62530a` reads the template one hop
/// earlier than `0x5efe00` does and lands in the same three values).
/// `& 4` → 0, `& 2` → 1, else −1. This crate holds no DBC (decision 0068 §3), so the walk is the
/// app's and the answer arrives pre-resolved as [`UnitState::pvp_team`](super::UnitState).
///
/// **It is not [`UnitState::faction_group`](super::UnitState), which this used to read.** That
/// field is `UnitFactionGroup`'s — the unit's LIVE `UNIT_FIELD_FACTIONTEMPLATE` — and the two
/// part company the moment anything moves a unit off its racial faction. A vmangos GM is forced
/// to template 35 (group mask 0), so every rank title in the Honor tab read `NONE` for a Grand
/// Marshal while the 1.12 client on the same server read "Grand Marshal" off his unchanged race:
/// report B378, decision 2227.
///
/// −1 needs no special case: it formats into the key, and `PVP_RANK_9_-1` matches no GlobalString.
fn team_of(u: &super::UnitState) -> i64 {
match u.faction_group.as_deref() {
Some("Horde") => 0,
Some("Alliance") => 1,
_ => -1,
}
i64::from(u.pvp_team)
}
/// `GetPVPRankInfo`'s **second argument** → the team digit, the engine's three-way dispatch
@@ -399,11 +399,11 @@ fn team_arg(lua: &Lua, v: Value) -> mlua::Result<i64> {
.and_then(|t| model.unit(t))
// `0x51a9af`'s `shr edx,4; test dl,1` player gate. Applied here, unlike in `UnitPVPRank`
// (see there), because on this arm nothing else can tell a creature from a player: a
// PvP-flagged city guard's faction template carries Player|<side> too, so its
// `faction_group` would otherwise name a side the engine answers 0 for. The cost is the
// mirror-image miss — an un-enriched player snapshot reads as a non-player — which lands
// on a different title rather than on none, and this arm is addon surface: neither pane
// passes a second argument at all.
// creature carries a race byte and a faction template like anything else, and the two
// misses are different numbers anyway (this gate's 0 against `0x5efe00`'s −1) — it is
// this gate the engine reaches first. The cost is the mirror-image miss — an un-enriched player snapshot reads as a non-player — which
// lands on a different title rather than on none, and this arm is addon surface: neither
// pane passes a second argument at all.
.filter(|u| u.is_player)
.map_or(0, team_of))
}
crates/benilla-ui/src/script/pvp/tests.rs
+64 -3
View File
@@ -64,7 +64,12 @@ fn player() -> UnitState {
name: Some("Benilla".into()),
sex: 3, // female
is_player: true,
// Both, because a real Alliance player has both — and they are not the same field.
// `faction_group` is `UnitFactionGroup`'s live faction template; `pvp_team` is
// `0x5efe00`'s race walk, which is what every rank title is keyed by. The GM-mode test
// below is the one that drives them apart.
faction_group: Some("Alliance".into()),
pvp_team: 1,
pvp_rank: 9,
..Default::default()
}
@@ -337,6 +342,7 @@ fn the_pane_title_is_keyed_by_team_and_is_never_gendered() {
Some(UnitState {
sex: 2,
faction_group: Some("Horde".into()),
pvp_team: 0,
..player()
}),
);
@@ -417,7 +423,7 @@ fn a_missing_rank_global_reads_nil_not_empty() {
s.set_unit(
"player",
Some(UnitState {
faction_group: None,
pvp_team: -1,
..player()
}),
);
@@ -425,6 +431,59 @@ fn a_missing_rank_global_reads_nil_not_empty() {
assert!(s.eval::<bool>("return GetPVPRankInfo(9) == nil").unwrap());
}
/// **The team digit is NOT the faction group** — report B378 at the binding level (decision 2227).
///
/// `UnitFactionGroup` reads the unit's live `UNIT_FIELD_FACTIONTEMPLATE` (`0x516630`) and the rank
/// title's team digit reads the unit's RACE (`0x5efe00`), so a vmangos GM — template 35, group
/// mask 0 — has no side and keeps his rank. Both `0x5efe00` surfaces are asserted here: the
/// binding's key and `UnitPVPName`'s decoration (`0x5efe60`, the same walk).
#[test]
fn a_sideless_player_still_has_a_team_digit_because_his_race_has_one() {
let mut s = seated();
seat_rank_globals(&s);
s.lua()
.globals()
.set("UNIT_PVP_NAME", "%s %s")
.expect("global");
s.set_unit(
"player",
Some(UnitState {
// `.gm on` — the template names nothing …
faction_group: None,
faction_group_localized: None,
// … and the race still names Alliance.
pvp_team: 1,
sex: 2,
pvp_rank: 18,
..player()
}),
);
assert_eq!(
s.eval::<String>("return (GetPVPRankInfo(18))").unwrap(),
"Grand Marshal",
"the sideless template must not reach the key"
);
assert_eq!(
s.eval::<String>(r#"return UnitPVPName("player")"#).unwrap(),
"Grand Marshal Benilla",
"and the name decoration reads the same digit"
);
// The control: a unit whose RACE resolves to nothing is still −1, and −1 still misses.
s.set_unit(
"player",
Some(UnitState {
faction_group: Some("Alliance".into()),
pvp_team: -1,
pvp_rank: 18,
..player()
}),
);
assert!(
s.eval::<bool>("return GetPVPRankInfo(18) == nil").unwrap(),
"a side on the template cannot stand in for a missing team digit either"
);
}
/// The internal→visual conversion across its whole range — **and the negative half runs the
/// opposite way from the server's `visualRank`**, which is the assertion that matters: rank 1 is
/// −4 and rank 4 is −1, because `0x51aa38` subtracts 5 rather than negating. vmangos's
@@ -520,6 +579,7 @@ fn get_pvp_rank_info_takes_a_second_argument_three_different_ways() {
name: Some("Thrall".into()),
is_player: true,
faction_group: Some("Horde".into()),
pvp_team: 0,
pvp_rank: 14,
..Default::default()
}),
@@ -575,7 +635,7 @@ fn get_pvp_rank_info_takes_a_second_argument_three_different_ways() {
Some(UnitState {
exists: true,
name: Some("Timber Wolf".into()),
faction_group: Some("Alliance".into()),
pvp_team: 1,
..Default::default()
}),
);
@@ -593,7 +653,7 @@ fn get_pvp_rank_info_takes_a_second_argument_three_different_ways() {
Some(UnitState {
exists: true,
is_player: true,
faction_group: None,
pvp_team: -1,
..Default::default()
}),
);
@@ -643,6 +703,7 @@ fn unit_pvp_rank_answers_for_a_foreign_unit() {
name: Some("Thrall".into()),
is_player: true,
faction_group: Some("Horde".into()),
pvp_team: 0,
pvp_rank: 14,
..Default::default()
}),
crates/benilla-ui/src/script/unit/mod.rs
+18
View File
@@ -341,6 +341,24 @@ pub struct UnitState {
/// return, which stock uses as display text (`PlayerFrame.lua`'s PvP hit-area tooltip title).
/// Never interchangeable with the English half above.
pub faction_group_localized: Option<String>,
/// The unit's **PvP team digit** — `0x5efe00`'s tri-state: `0` Horde, `1` Alliance, `-1` a
/// unit with no side. The second `%d` of `PVP_RANK_<rank>_<team>`, and **not**
/// [`Self::faction_group`] restated.
///
/// The two answer different questions and the difference is a shipped bug's whole cause
/// (report B378, decision 2227): `UnitFactionGroup` reads the unit's LIVE
/// `UNIT_FIELD_FACTIONTEMPLATE` (`0x5166b8`/`0x5166be`), while every rank-title surface reads
/// the unit's **RACE** and walks `ChrRaces` → `FactionTemplate` → factionGroupMask
/// (`0x5efe00`, `[obj+0x110]+0x78`). A vmangos GM is forced to template 35 and so genuinely
/// loses the PvP flag icon — and keeps his rank title, because his race did not move. Wiring
/// this to the faction group instead read `NONE` at every rank for a Grand Marshal.
///
/// **`Default` is `0`, and that is the reference's answer too, not a placeholder.** A literal
/// snapshot is one the object manager could not resolve (the out-of-range roster leg), and
/// `GetPVPRankInfo`'s team register is left at its initial `0` on exactly that edge
/// (`0x51a9af`'s lookup failing) — so a roster-only unit is named off the Horde list on both
/// clients. Every snapshot built from a live descriptor fills it (`ui_unit::snapshot`).
pub pvp_team: i8,
/// The unit's GUID (`OBJECT_FIELD_GUID`) — the identity the cross-token predicates compare
/// (`UnitIsUnit`, `UnitInParty`; decision 0434 §5's popup gating). `0` = the app's feed didn't
/// resolve one; two zero guids never compare equal.