Skip to content

Fix: broken link to refactoring requirements #372

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.

Sign up for GitHub

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

Closed
suspect15 wants to merge 2 commits into from
Closed

Fix: broken link to refactoring requirements #372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
487e3a8
Fix: broken link to refactoring requirements
suspect15 May 27, 2026
3ef79a5
Fixed omitted "definition" and added static address requirements
suspect15 May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/.vitepress/config/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ function sidebarGuidelines(): DefaultTheme.SidebarItem[] {
},
{
text: "Refactoring Requirements",
link: "/guidelines/content/refactoring-requirements",
link: "/guidelines/content/refactor-requirements",
},
],
},
Expand Down
26 changes: 24 additions & 2 deletions docs/guidelines/content/refactor-requirements.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,30 @@ The following aspects of a set must all be up to modern standards in order to be

## Code Notes

Code notes must, at a minimum, contain the address's size, a description of what the address does or why it is used in the achievement code, and enumerated values and their associated.
- Values must be in hexidecimal or float format, listed in increasing value by default
Refactored sets will follow a strict format standard that must be adhered to. Code notes must, at a minimum, contain the address's size, a description of what the address does or why it is used in the achievement code, and enumerated values and their associated definitions.

**Static addresses**

Static addresses shall be formatted as follows:

- Bracketed size at the beginning of every note, may include BE or BCD as appropriate [16-bit BE], [16-bit BCD], [16-bit BE BCD]
Copy link
Copy Markdown
Contributor

@redwizard42 redwizard42 May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will likely never refactor a set under these rules if you make me put it at the very beginning.

Copy link
Copy Markdown
Contributor

@redwizard42 redwizard42 May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be quite this strict. I think that's a bad move.

Copy link
Copy Markdown
Contributor

@redwizard42 redwizard42 May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Bracketed, on the first line of the note, at beginning or end" would much better match the current modern code note ecosystem (but better worded), IMO.

Copy link
Copy Markdown
Contributor Author

@suspect15 suspect15 May 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA discussed this and decided that a single format code note standard for refactoring is preferred over individual stylistic preferences. Many of us expressed difficulty parsing notes with sizes in unpredictable locations due to uncertain lengths of preceding descriptions. With prepended, bracketed sizes, we felt that it is very easy to navigate notes with an extremely small variance in indentation prior to a description - a character or two in the vast majority of cases.

One of the primary goals of the refactoring project is to transform old, haphazard sets into very consistent packages that future maintainers can easily work through without any presentation variation. We want to minimize any personal style preference and have these sets effectively archived in perpetuity as standardized as possible. We felt that the size consistently up front was the overall best long term solution.

That said, as someone whose resolved hundreds upon hundreds of inactive dev tickets and has put in a ton of maintenance work, I consider your perspective on this especially valuable. I'll bring that up to the team and discuss it again. My personal priority is more standardization than my preference about whether size goes up front or at the back.

Copy link
Copy Markdown
Contributor

@laytonloztew laytonloztew May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally prefer size up front, but I think a consistent and reasonable style should be required rather than a specific one. A set where some notes have size at beginning and some have size at end is difficult to parse, but having all the notes one way or the other doesn't make a difference.

The docs says "A refactored set should be able to pass a code review without needing corrections.", but I wouldn't require a jr to rewrite all their notes to have the size at the beginning - I would require they be consistent with a format that most devs can understand.
Bitflags not being bracketed is another that would dissuade me from refactoring, I find using [8-bit] is less clear than [Bitflags] or [Bitfield]. Its like marking a float as [32-bit] and saying that the decimal values make it clear its actually a float. And I'd argue that bitflag notes should never be more than 8-bit anyway.

What I'm afraid you'll see are devs fixing sets to their liking and not informing QA, so the set stays in the refactor hub and its more difficult to find sets that actually need refactoring. Its already a tough sell when I have to do extra work (fixing code notes, making save states) beyond just fixing the set for players and preventing further tickets.

Copy link
Copy Markdown
Contributor Author

@suspect15 suspect15 May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to close this PR. Someone else can run with this if they want.

- Bitflags shall not be bracketed, but may be included in the note description, not required though as seeing values as Bits makes it clear that the address contains bitflags
- Address description in clear, concise verbiage - may expand as needed, but should not unnecessarily
- Values listed either in hex or float depending on address type. Should be increasing in order unless out of order makes sense for something like Map ID progression where the IDs are not ordered sequentially
- Values must use an = sign, however spacing is optional: no spaces, space before/after =, or on both sides

```
[16-bit BE BCD] Description
0x0000=Value 1
...
0x0001=Value 2
```
```
[8-bit] Event bitflags
Bit0 = Something occurred
...
Bit7 = Something else occurred
```

## Achievement Logic

Expand Down
Loading