-
Notifications
You must be signed in to change notification settings - Fork 85
Add: Refactor requirements doc #383
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.
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
Draft
suspect15
wants to merge
1
commit into
RetroAchievements:main
Choose a base branch
from
suspect15:refactor-reqs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+98
−0
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| --- | ||
| title: Refactoring Requirements | ||
| description: This guide covers the requirements for refactoring a substandard, inactive developer's set. | ||
| --- | ||
|
|
||
| # Refactoring Substandard Sets | ||
|
|
||
| [[toc]] | ||
|
|
||
| # Refactoring Overview | ||
|
|
||
| Refactoring a set on RetroAchievements is the act of bringing a substandard, inactive developer set up to modern standards. Ridding the library of poorly noted, coded, and implemented sets is the only way RetroAchievements can attain longterm stability. This doc provides guidance on what is required to bring a set up to modern standards and consider it no longer an instability risk to the project. Once a refactor is completed in accordance with these guidelines, the set can be marked by QA as "refactored" and should be considered stable in perpetuity. | ||
|
|
||
| **A refactored set should be able to pass a code review without needing corrections.** | ||
|
|
||
| # Finding a Set to Refactor | ||
|
|
||
| Only sets that are in the [Needs Refactoring hub](https://retroachievements.org/hub/3389) are eligible to be refactored. | ||
|
|
||
| # The Refactoring Process | ||
|
|
||
| The following aspects of a set must all be up to modern standards in order to be considered refactored and no longer in need of additional improvement: | ||
| - Code Notes | ||
| - Achievement Logic | ||
| - Leaderboard Function and Logic | ||
| - Title and Description Writing | ||
| - Rich Presence | ||
|
|
||
| ## Code Notes | ||
|
|
||
| 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 bit, hex, or float values only 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] | ||
| - 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 | ||
| ... | ||
| 0x000X=Value X | ||
| ``` | ||
| ``` | ||
| [8-bit] Event bitflags | ||
| Bit0 = Something occurred | ||
| ... | ||
| Bit7 = Something else occurred | ||
| ``` | ||
|
|
||
| ## Achievement Logic | ||
|
|
||
| Achievement logic must be free of bad practices such as lack of Mem/Delta checks, unnecessary use of hits, resets, and pauses, and unnecessarily complex or extraneous logic. | ||
| - Many older sets were inadequately RAM dug and will likely benefit from or require additional RAM digging in order to ensure proper addresses are used | ||
|
|
||
| ## Leaderboard Function and Logic | ||
|
|
||
| Any leaderboard that submits a value tracked in-game should be instantaneous, not primed, unless there is an exceptionally compelling reason. Instantaneous leaderboards reduce screen clutter and are much more simple to code and maintain. Leaderboard logic is held to the same expectations as achievement logic. | ||
|
|
||
| ## Title and Description Writing | ||
|
|
||
| At a minimum, gross errors such as miscapitalization, unnecessary parentheticals, and grammar mistakes should be corrected. If writing is generally poor, the set should be referred to the [WritingTeam](https://retroachievements.org/user/WritingTeam) so they can put it into the [Noncompliant Writing hub](https://retroachievements.org/hub/24397). | ||
|
|
||
| ## Rich Presence | ||
|
|
||
| Rich Presence must be dynamic and free of useless information. Unclear emojis and superfluous language should be removed to ensure RP is clear, concise, and understandable by site viewers who may not be particularly familiar with the game. | ||
|
|
||
| # Save States | ||
|
|
||
| Collecting save states during the refactoring process will make future maintenance significantly easier. Eventually on site storage may be available, so collecting complete sets of save states is highly desired during the refactoring process. Save states should use the most [recommended core](https://docs.retroachievements.org/general/emulator-support-and-issues.html) and its most current version. | ||
|
|
||
| States should be collected so all achievements are easy to access and reported issues should be easily repeatable. In general, the following are the states that should be collected: | ||
|
|
||
| - Prior to any boss fights that have associated achievements for progression, collection, or challenge | ||
| - Prior to obtaining the last item, skill, or level for collection/power-up achievements | ||
| - Prior to the beginning of a challenge's initial checkpoint hit | ||
| - Periodically at sensible points throughout standard gameplay to give general access to all parts of the game | ||
|
|
||
| # Refactor Notification | ||
|
|
||
| Upon fully completing a refactoring, developers should notify [QATeam](https://retroachievements.org/user/QATeam) via site message. QA will confirm the set meets full refactor criteria and remove it from the Needs Refactoring hub. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sentence here ends abruptly and feels like more words should follow "but should not unnecessarily"
I think the intent from context is to not unnecessarily expand description