226 lines
16 KiB
Markdown
226 lines
16 KiB
Markdown
# Contributing
|
||
|
||
benilla is a faithful 1.12.1 client. It is the foundation people build on, not the place to get
|
||
creative: a change is accepted when it makes benilla more like 1.12.1 or fixes a bug, with
|
||
evidence from the reference, in one small piece, with the gates green. Everything else lives on
|
||
top of benilla, in a crate of its own or a fork, and both are welcome.
|
||
|
||
## Where to start
|
||
|
||
The [issues](https://github.com/samwhosung/benilla/issues) the maintainers file are checked
|
||
against 1.12.1 first: each says what 1.12.1 does, what benilla does instead, and where in the
|
||
code, and any of them is a place to start. One labelled `in progress` is taken: someone is
|
||
working on it or a pull request is open for it, so pick another. Once you start one, open a draft
|
||
pull request that says `Fixes #N` as soon as you have a first commit, so the issue shows it is
|
||
taken.
|
||
|
||
## What gets in
|
||
|
||
- A fix for a bug, with how to see it before and after.
|
||
- A step closer to 1.12.1: a missing packet, verb, window, effect or behaviour, done the way
|
||
the real client does it.
|
||
- A correction where benilla and the reference disagree, with the reference fact stated.
|
||
- Whatever the stock 1.12.1 client loads from a patch, benilla loads: models, textures or data
|
||
rows that 1.12.1 shows and benilla does not are a bug.
|
||
|
||
## What does not
|
||
|
||
- Features 1.12.1 does not have, including what 1.12 client mods add (a new key binding, a spell
|
||
queue, new Lua functions, graphics past 1.12's), and behaviour changed because it seems better.
|
||
They live on top of benilla ("Building on top"), so that what benilla does is what 1.12.1 does,
|
||
for everyone who builds on it. A deviation from the reference is the maintainer's call and is
|
||
recorded where it lives, and gathered in [`docs/DEVIATIONS.md`](DEVIATIONS.md); a pull request is
|
||
not the place to propose one.
|
||
- Anything from a WoW install: art, models, sounds, maps, data, and interface code (FrameXML and
|
||
GlueXML), which runs off the player's own install (`docs/METHOD.md`).
|
||
- Big or mixed changes. One change per pull request, small enough to read in one sitting.
|
||
|
||
## Building on top
|
||
|
||
A feature 1.12.1 does not have lives in a crate of its own that adds its Bevy plugins through
|
||
`benilla_app::run_with`, as `crates/benilla-app/examples/extended_launcher.rs` shows, or in a
|
||
fork. A client started through `run_with` says `extended` in its build line, so its reports read
|
||
apart from stock benilla's. The crate keeps its own settings, never rows in benilla's CVar table,
|
||
which every addon reads, and its files go under `benilla_app::config_dir()`, its own
|
||
`benilla-config/`: an addon it ships is written into that folder's `AddOns/` and loads as any
|
||
addon does.
|
||
|
||
Past `run_with` and that folder, benilla opens a piece of itself to such a crate only when its own
|
||
1.12.1 work creates that piece, never for the crate alone, and promises no stable API: a crate
|
||
pins the benilla revision it builds on. What a crate cannot reach stays a fork's.
|
||
|
||
## The reference
|
||
|
||
[benilla-re](https://github.com/samwhosung/benilla-re) is the reverse engineering of the 1.12.1
|
||
client that benilla is built against, read at its bytes, and the place to find what 1.12.1 does
|
||
before you change benilla. Every client address a comment here cites is backed there, by a row
|
||
or a finding.
|
||
|
||
- **Look it up.** The site, `https://samwhosung.github.io/benilla-re/`, has a page per function,
|
||
global and struct, per system, and per Lua function, event, CVar and opcode;
|
||
`?0x5e6853` goes to whatever holds an address. For an agent, a clone beside this repo is
|
||
quicker: grep `symbols/functions.tsv` (a row per function: name, meaning, confidence, the
|
||
finding behind it), `tables/` (opcodes, events, CVars, Lua bindings, messages) and the docs
|
||
under `systems/`.
|
||
- **Weigh it.** A row says how sure it is: `inferred` is a lead to check before you rest on it,
|
||
`verified` with proof `one` or `two` is one or two readings of the bytes.
|
||
- **When it has a gap or is wrong,** take that to benilla-re: an issue there with what you need to
|
||
know, or a pull request that adds the finding (its `docs/CONTRIBUTING.md` says how a finding is
|
||
written and checked). Then cite the client address here, as the comments do, and link that
|
||
issue or pull request in yours while it is open.
|
||
- **The vendored tables.** `reference/` holds the benilla-re tables benilla's tests read, at the
|
||
commit in `reference/PIN`; its README says how to move the pin. Move it only when your change
|
||
needs a row benilla-re has after the pin, to a commit on benilla-re's main.
|
||
|
||
## How a change is judged
|
||
|
||
1. `scripts/gates.sh` is green: fmt, clippy with warnings denied, the workspace tests (once
|
||
with the client data, once without, so a test that reads the install has to declare it), the
|
||
doc-link and render-pass lints, the player build with its own tests, and the engine boot
|
||
checks. A pull request from a fork runs the ones that need neither the install nor a display
|
||
in CI, on Linux; the rest run on the maintainer's machine before it lands.
|
||
2. The reference fact is stated: what 1.12.1 does, and where that is known from (an entry in
|
||
[benilla-re](https://github.com/samwhosung/benilla-re) by client address, the client's
|
||
behaviour you observed, a DBC field, a FrameXML line, a packet capture). The names and shapes
|
||
under `reference/` are the surface benilla tracks. A fact benilla-re does not have yet is
|
||
added there first ("The reference").
|
||
3. A comment where it matters, one line, saying what the code does and the 1.12 fact behind it.
|
||
No history.
|
||
4. The commit message says what changed, for a player or a developer, in one line.
|
||
|
||
## After you open it
|
||
|
||
A maintainer reads it, checks it against the reference, runs it and finishes it here: a rebase
|
||
onto main, a fix, a test or a comment, added as commits on top of yours on your branch, so leave
|
||
"Allow edits by maintainers" ticked. It lands as one squash-merged commit with you as its author.
|
||
When what would land is mostly ours, we land our own version with you as a co-author and close
|
||
yours with a note, and when main already has the fix, we close yours and say where. One that is
|
||
out of scope is closed with the reason.
|
||
|
||
## Setting up
|
||
|
||
- **The toolchain.** Stable Rust (`rust-toolchain.toml` adds clippy and rustfmt) and a C
|
||
compiler, which some dependencies build with. On macOS the Xcode
|
||
command line tools, which also supply libclang for the audio bindings; on Linux the ALSA and
|
||
udev development packages and pkg-config. `python3` runs two of the gates.
|
||
- **A 1.12.1 install of your own.** `WOW_DATA=<its Data folder>`, or a `WoW` link at the repo
|
||
root, which only a dev build sees: the player build looks for `Data/` or `WoW/Data/` beside
|
||
the binary. benilla reads the install and never writes into it. `WOW_DATA=` (set, empty)
|
||
means "no install": the client refuses to start (an error dialog, or stderr alone in an
|
||
unattended run) and the no-install tests run on a machine that has one.
|
||
- **Without the install, green is hollow.** Well over a thousand tests read the install and skip
|
||
when it is absent, and a few dozen more read a corpus of vanilla addons
|
||
(`BENILLA_ADDON_CORPUS=<a folder of addons>`, or a `wow-addons-vanilla` link at the root),
|
||
third-party content that is not in this repo. `scripts/gates.sh` and `scripts/check.sh` print
|
||
how many tests skipped and why. Where the data is, `BENILLA_REQUIRE_DATA=1` turns a skip into a
|
||
failure, and the gates set it themselves when the install and the corpus both resolve.
|
||
- **A server to test against.** Any 1.12.1 server; `WOW_HOST` names it (default
|
||
`localhost:3724`). A scripted run has no default account: `WOW_USER`, `WOW_PASS` and
|
||
`WOW_CHAR` name a test account on your server whose login kicks nobody, all three, either in
|
||
the environment or in a `.probe-identity` file at the repo root (one per line, never
|
||
committed), and `scripts/smoke.sh` (the live login gate) refuses without them. A linked
|
||
worktree shares the main checkout's `benilla-config/`, unless it has a `.probe-identity`: then
|
||
it keeps its own, so no run from it touches your settings. `WOW_CHAR` must already be on the
|
||
account, since the client creates a character only when asked:
|
||
`WOW_PROBE_CHARCREATE=<name> WOW_PROBE_CHARCREATE_KEEP=1`, run without `WOW_CHAR`, makes it.
|
||
The probes drive the body with GM commands, so give that account the top GM level.
|
||
- **Running it unattended.** The rules are `docs/METHOD.md`, "The local server"; these are the
|
||
switches.
|
||
- `WOW_UNATTENDED=1` reconnects instead of waiting at a dialog, and exits non-zero on a login
|
||
it cannot pass. `WOW_NOSOUND=1` runs silent. A capture (`WOW_CAPTURE`) is both, and a rig
|
||
(`WOW_RIG`) is unattended. `WOW_ALLOW_ACCOUNT=1` overrides the account guard.
|
||
- A PIN-protected account needs one more variable, which answers the keypad's prompt in place
|
||
of a key press. Set one: `WOW_PIN` is the PIN's 4 to 10 digits, sent as typed;
|
||
`WOW_PIN_TOTP` is a base32 secret, whose current RFC 6238 code (HMAC-SHA1, 30 s step,
|
||
6 digits) is the PIN, the scheme vmangos checks. vmangos reads a code as a number, so a code
|
||
with leading zeros goes without them (`081804` is the five digits `81804`). Both set is an
|
||
error: it is logged and the logon fails. Neither value is ever logged. The scripts under
|
||
`scripts/` clear inherited `WOW_*` variables and do not carry these.
|
||
- An unattended run starts from defaults in a fresh state folder of its own, printed at boot
|
||
(`state:`), so it never reads or overwrites your settings, layout or addons. `BENILLA_HOME`
|
||
pins a folder, for a run that needs addons, seeded files or state kept across runs.
|
||
- On vmangos, `WOW_RIG="tauren druid 60 gear:heal-preraid-bis spec:heal-preraid-bis
|
||
at:ThunderBluff"` finds or creates that character on the account and applies the server's
|
||
premade sets (`WOW_RIG="gear:?"` lists them). `WOW_PROBE_CHAT="<command>"` sends one GM
|
||
command and logs the reply as `net: server says`. An account named `probe` and digits is
|
||
shielded: every world entry sends `.cheat god on`, `.die` clears it for a death test, and
|
||
`WOW_GOD=off` leaves it off.
|
||
- A rig, `WOW_PROBE_CHARCREATE` and a glue capture drive the stock glue screens through their
|
||
own Lua, the calls a player's clicks make.
|
||
- A trace is `WOW_MOVE_TRACE=<file>`, filtered with `WOW_MOVE_TRACE_TAGS` (`"move,snd,in"`) and
|
||
ended by `WOW_PROBE_EXIT_AT=<seconds>`; grep it for the `rly` and `snd` lines.
|
||
- To sniff the wire, trace the tags `in` and `out` (`WOW_MOVE_TRACE_TAGS="in,out"`): one line
|
||
per world packet, with its opcode, name, `len=` and the body as `hex=`, the bytes after the
|
||
packet header. An `in` line also has `ev=`, the events it decoded to; a server packet that
|
||
failed to parse is a `skip` line with its bytes. Traces get attached to public issues, so two
|
||
kinds of body are kept out. A client packet that carries a secret, the auth session, a channel
|
||
password or any chat text starting with `.` or `!` (a server command), shows
|
||
`hex=<redacted>` always, and so does a dropped command's log line. Chat in both directions
|
||
and the bulk name lists (`SMSG_WHO`, the friend, ignore, guild roster and channel lists) show
|
||
`hex=<withheld>`, with `len=` kept, unless `WOW_MOVE_TRACE_CHAT=1` is set, for a developer who
|
||
needs those bodies and will read the file before sharing it.
|
||
- A capture runs through `scripts/visual.sh`, whose recipe is the header of
|
||
`crates/benilla-app/src/capture/mod.rs`. Pixel questions go through `benilla-visual crop`,
|
||
`series` and `hotspot`.
|
||
- **Asking the UI without a server.** The addon harness's probe loads the interface into a VM
|
||
with no window, server or login, and answers in under a second. Build it once with
|
||
`cargo build --profile play -p benilla-app --example addon_harness`, then run
|
||
`target/play/examples/addon_harness`:
|
||
- `<folder> --probe <Name> [<step>]…` loads the stock FrameXML, benilla's layer and one addon
|
||
from a folder of addons, with its dependencies from the same folder.
|
||
`--probe-ui [<step>]…` loads the interface with no addon, for work on FrameXML or the layer.
|
||
- Each run seats one session: a level 60 Human Warrior named `Harness` with a target, a buff,
|
||
an action on cooldown, a spellbook and a quest log. It fires the addon's `ADDON_LOADED` (none
|
||
under `--probe-ui`), then `VARIABLES_LOADED`, `PLAYER_LOGIN` and `PLAYER_ENTERING_WORLD`,
|
||
then ticks one second. It reads and writes no saved variables.
|
||
- The steps run in the order given. `--eval <lua>` answers its first return value through
|
||
`tostring`, so return a string to read more than one value. `--event <NAME> [<arg>]…` fires
|
||
an engine event; an argument that reads as a number is a number, and `nil` is nil.
|
||
`--mouse <x>,<y>` hovers, `--click <x>,<y>[,<Button>]` presses and releases, and
|
||
`--tick <secs>` runs one `OnUpdate` frame. Points are UI units on a 1024×768 screen, from
|
||
the bottom left.
|
||
- An `--eval` answers `= <value>`, `ERROR: <message>` for a raise, or `SYNTAX: <message>`.
|
||
The other steps answer what their handlers raised, `= no errors` or `= RAISED: <message>`,
|
||
after the frame under the cursor for `--mouse` and `--click`. An event name 1.12 lacks
|
||
answers `ERROR`, since no frame can register for it. Load and session errors print first.
|
||
- A window that waits on server state (a bank, a mailbox, a raid) has nothing to show here.
|
||
Ask a live session with `WOW_PROBE_LUA` instead.
|
||
- **The loop.** `cargo play` builds and runs the play profile. `cargo test -p <crate>` runs one
|
||
crate's tests, the quickest loop. `scripts/check.sh` verifies a round of work: the crates your
|
||
branch changed against main and their dependents, so on a clean clone it has nothing to check; `scripts/gates.sh` is the full chain, and it opens a window for the engine boot
|
||
checks, so it needs a display. `WOW_STOCK_UI=1` boots a dev build on the stock interface
|
||
alone, without benilla's layer. Work on a branch.
|
||
|
||
## Recording frame costs
|
||
|
||
In a player or dev build, `/console fpsJournal 1` starts a recording; reproduce the slow scene
|
||
for about a minute, then `/console fpsJournal 0` stops it. Send all three files from
|
||
`benilla-config/Diagnostics/`: `fps-journal.csv`, `gpu-passes.log` and `cpu-phases.csv`. No
|
||
environment variable or launcher console is needed. Recordings append, with session markers
|
||
to distinguish restarts. `WOW_FPS_JOURNAL=<csv path>` puts the companion files beside that CSV.
|
||
|
||
The GPU log reports each top-level render pass in milliseconds per readback frame, summing
|
||
repeated views; unsupported or missing GPU measurements say `unavailable`. The CPU file
|
||
reports complete frames by schedule, with wall time, whole-process CPU time and main-thread
|
||
CPU time separately. `BetweenFrames` includes presentation pacing, the event loop and the
|
||
render handshake. Process CPU includes concurrent worker and render threads, so it can exceed
|
||
wall time and is not exclusive attribution to that schedule. These are phase measurements,
|
||
not sampled function stacks; clock reads add overhead only while recording. Blank CPU cells
|
||
mean that platform's clock is unavailable, not zero work. On Windows the CPU file counts
|
||
thread cycles, since its tick-sized process times cannot split a frame; its cells stay blank
|
||
for the first second after launch while that clock calibrates.
|
||
|
||
A crate on top labels a recording with its own state through `benilla_app::FpsJournalTags`:
|
||
each tag is written into all three files as a `# session=… t=… tag key=value` line when the
|
||
recording starts and whenever it changes, so toggling a feature mid-recording splits the
|
||
seconds by it. `scripts/journal-report.py <fps-journal.csv>` summarises a recording per session
|
||
and tag state, side by side: frame and GPU columns, every GPU pass, and the CPU phases.
|
||
|
||
## Reporting a bug
|
||
|
||
Open an issue: what you did, what you saw and what 1.12.1 does instead, with the `benilla build`
|
||
line from the start of the terminal output, your platform and the server you ran on. Questions
|
||
and ideas are welcome there or on the Discord linked from the README.
|
||
|
||
Working with an AI agent is expected. The agent reads `AGENTS.md`, and the same rules bind it.
|