From 2e6c62ff54b8efd3a81d4d896aed3fb646c68606 Mon Sep 17 00:00:00 2001 From: psf <77138753+pkmnsnfrn@users.noreply.github.com> Date: Tue, 1 Apr 2025 09:38:59 -0400 Subject: [PATCH] Add CONTRIBUTING.md and STYLEGUIDE.md (#6340) Co-authored-by: HackMD <37423+hackmd-hub[bot]@users.noreply.github.com> Co-authored-by: Alex <93446519+AlexOn1ine@users.noreply.github.com> Co-authored-by: hedara90 <90hedara@gmail.com> --- .../ISSUE_TEMPLATE/01_battle_engine_bugs.yaml | 33 +- .../ISSUE_TEMPLATE/02_battle_ai_issues.yaml | 33 +- .../ISSUE_TEMPLATE/03_feature_requests.yaml | 23 +- .github/ISSUE_TEMPLATE/04_other_errors.yaml | 33 +- .github/pull_request_template.md | 49 ++- CONTRIBUTING.md | 143 +++++++ docs/CONTRIBUTING.md | 1 + docs/CREDITS.md | 1 + docs/STYLEGUIDE.md | 399 ++++++++++++++++++ docs/SUMMARY.md | 2 + docs/team_procedures/merge_checklist.md | 66 +++ 11 files changed, 733 insertions(+), 50 deletions(-) create mode 100644 CONTRIBUTING.md create mode 100644 docs/CONTRIBUTING.md create mode 100644 docs/CREDITS.md create mode 100644 docs/STYLEGUIDE.md create mode 100644 docs/team_procedures/merge_checklist.md diff --git a/.github/ISSUE_TEMPLATE/01_battle_engine_bugs.yaml b/.github/ISSUE_TEMPLATE/01_battle_engine_bugs.yaml index 7d3ad81846..b570c6f322 100644 --- a/.github/ISSUE_TEMPLATE/01_battle_engine_bugs.yaml +++ b/.github/ISSUE_TEMPLATE/01_battle_engine_bugs.yaml @@ -5,23 +5,42 @@ body: - type: markdown attributes: value: | - Please fill in all required fields with as many details as possible. + Please fill in all fields with as many details as possible. - type: textarea id: description attributes: label: Description description: | - Describe the issue you are experiencing. - Attach images/videos if possible. + What behavior are you expecting to happen? What behavior are you observing instead? placeholder: | - Please enter a description of the issue. Here you can also attach log screenshots, gifs or a video + Please be as descriptive as possible. validations: required: true + - type: textarea + id: reproduction + attributes: + label: Reproduction Steps + description: | + What exact steps can somebody else follow in order to recreate the issue on their own? + placeholder: | + Provide as much context as possible as to what was done to create the issue. + validations: + required: true + - type: textarea + id: media + attributes: + label: Images / Video + description: | + Do you have images or videos to show the problem happen? + placeholder: | + Here you can also attach logs, screenshots, gifs or a video. + validations: + required: false - type: dropdown id: version attributes: label: Version - description: What version of pokeemerald-expansion are you using as a base? + description: What version of pokeemerald-expansion are you using? options: - 1.11.1 (Latest release) - master (default, unreleased bugfixes) @@ -38,14 +57,14 @@ body: id: upcomingversion attributes: label: Upcoming/master Version - description: If you're using the upcoming or master branches directly, please specify what was the commit hash you pulled from. + description: If you're using the `upcoming` or `master` branches directly, please use the following command to give us the commit hash that you are on. `git log --merges RHH/upcoming -1 --format=%H` Replace `upcoming` with `master` if you're using `master`. validations: required: false - type: input id: contact attributes: label: Discord contact info - description: Provide your Discord tag here so we can contact you in case we need more details. Be sure to join our server ([here](https://discord.gg/6CzjAG6GZk)). + description: Provide your Discord tag here so we can contact you in case we need more details. Discussion around **`pokeemerald-expansion`** happens in our [Discord server](https://discord.gg/6CzjAG6GZk). placeholder: ex. Lunos#4026 validations: required: false diff --git a/.github/ISSUE_TEMPLATE/02_battle_ai_issues.yaml b/.github/ISSUE_TEMPLATE/02_battle_ai_issues.yaml index 1fe3387f6b..e3d40589ea 100644 --- a/.github/ISSUE_TEMPLATE/02_battle_ai_issues.yaml +++ b/.github/ISSUE_TEMPLATE/02_battle_ai_issues.yaml @@ -5,23 +5,42 @@ body: - type: markdown attributes: value: | - Please fill in all required fields with as many details as possible. + Please fill in all fields with as many details as possible. - type: textarea id: description attributes: label: Description description: | - Describe the issue you are experiencing. - Attach images/videos if possible. + What behavior are you expecting to happen? What behavior are you observing instead? placeholder: | - Please enter a description of the issue. Here you can also attach log screenshots, gifs or a video + Please be as descriptive as possible. validations: required: true + - type: textarea + id: reproduction + attributes: + label: Reproduction Steps + description: | + What exact steps can somebody else follow in order to recreate the issue on their own? + placeholder: | + Provide as much context as possible as to what was done to create the issue. + validations: + required: true + - type: textarea + id: media + attributes: + label: Images / Video + description: | + Do you have images or videos to show the problem happen? + placeholder: | + Here you can also attach logs, screenshots, gifs or a video. + validations: + required: false - type: dropdown id: version attributes: label: Version - description: What version of pokeemerald-expansion are you using as a base? + description: What version of pokeemerald-expansion are you using? options: - 1.11.1 (Latest release) - master (default, unreleased bugfixes) @@ -38,14 +57,14 @@ body: id: upcomingversion attributes: label: Upcoming/master Version - description: If you're using the upcoming or master branches directly, please specify what was the commit hash you pulled from. + description: If you're using the `upcoming` or `master` branches directly, please use the following command to give us the commit hash that you are on. `git log --merges RHH/upcoming -1 --format=%H` Replace `upcoming` with `master` if you're using `master`. validations: required: false - type: input id: contact attributes: label: Discord contact info - description: Provide your Discord tag here so we can contact you in case we need more details. Be sure to join our server ([here](https://discord.gg/6CzjAG6GZk)). + description: Provide your Discord tag here so we can contact you in case we need more details. Discussion around **`pokeemerald-expansion`** happens in our [Discord server](https://discord.gg/6CzjAG6GZk). placeholder: ex. Lunos#4026 validations: required: false diff --git a/.github/ISSUE_TEMPLATE/03_feature_requests.yaml b/.github/ISSUE_TEMPLATE/03_feature_requests.yaml index 8d56216265..9b7b46c9e1 100644 --- a/.github/ISSUE_TEMPLATE/03_feature_requests.yaml +++ b/.github/ISSUE_TEMPLATE/03_feature_requests.yaml @@ -5,23 +5,32 @@ body: - type: markdown attributes: value: | - Please fill in all required fields with as many details as possible. + Please fill in all fields with as many details as possible. - type: textarea - id: description + id: behavior attributes: - label: Description + label: Behavior Description description: | - Describe the issue you are experiencing. - Attach images/videos if possible. + What is the current behavior? What behavior would you expect your feature request to provide? What other information can you provide to help your feature get implemented? placeholder: | - Please enter a description of the issue. Here you can also attach log screenshots, gifs or a video + Provide as much context as possible. validations: required: true + - type: textarea + id: media + attributes: + label: Images / Video + description: | + Have other projects or games solved this problem? Do you have images or video to show this happening? + placeholder: | + Here you can also attach logs, screenshots, gifs or a video. + validations: + required: false - type: input id: contact attributes: label: Discord contact info - description: Provide your Discord tag here so we can contact you in case we need more details. Be sure to join our server ([here](https://discord.gg/6CzjAG6GZk)). + description: Provide your Discord tag here so we can contact you in case we need more details. Discussion around **pokeemerald-expansion** happens in our [Discord server](https://discord.gg/6CzjAG6GZk). placeholder: ex. Lunos#4026 validations: required: false diff --git a/.github/ISSUE_TEMPLATE/04_other_errors.yaml b/.github/ISSUE_TEMPLATE/04_other_errors.yaml index 0f37baf0fc..0e467b4536 100644 --- a/.github/ISSUE_TEMPLATE/04_other_errors.yaml +++ b/.github/ISSUE_TEMPLATE/04_other_errors.yaml @@ -5,23 +5,42 @@ body: - type: markdown attributes: value: | - Please fill in all required fields with as many details as possible. + Please fill in all fields with as many details as possible. - type: textarea id: description attributes: label: Description description: | - Describe the issue you are experiencing. - Attach images/videos if possible. + What behavior are you expecting to happen? What behavior are you observing instead? placeholder: | - Please enter a description of the issue. Here you can also attach log screenshots, gifs or a video + Please be as descriptive as possible. validations: required: true + - type: textarea + id: reproduction + attributes: + label: Reproduction Steps + description: | + What exact steps can somebody else follow in order to recreate the issue on their own? + placeholder: | + Provide as much context as possible as to what was done to create the issue. + validations: + required: false + - type: textarea + id: media + attributes: + label: Images / Video + description: | + Do you have images or videos to show the problem happen? + placeholder: | + Here you can also attach logs, screenshots, gifs or a video. + validations: + required: false - type: dropdown id: version attributes: label: Version - description: What version of pokeemerald-expansion are you using as a base? + description: What version of pokeemerald-expansion are you using? options: - 1.11.1 (Latest release) - master (default, unreleased bugfixes) @@ -38,14 +57,14 @@ body: id: upcomingversion attributes: label: Upcoming/master Version - description: If you're using the upcoming or master branches directly, please specify what was the commit hash you pulled from. + description: If you're using the `upcoming` or `master` branches directly, please use the following command to give us the commit hash that you are on. `git log --merges RHH/upcoming -1 --format=%H` Replace `upcoming` with `master` if you're using `master`. validations: required: false - type: input id: contact attributes: label: Discord contact info - description: Provide your Discord tag here so we can contact you in case we need more details. Be sure to join our server ([here](https://discord.gg/6CzjAG6GZk)). + description: Provide your Discord tag here so we can contact you in case we need more details. Discussion around **`pokeemerald-expansion`** happens in our [Discord server](https://discord.gg/6CzjAG6GZk). placeholder: ex. Lunos#4026 validations: required: false diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 9a04e86f2b..0b214dce0a 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -1,36 +1,41 @@ - + - - - + + + + + + + + + + + ## Description - - + -## Images - - +## Media + ## Issue(s) that this PR fixes - - + -## **People who collaborated with me in this PR** - - +## People who collaborated with me in this PR + + - + ## Feature(s) this PR does NOT handle: - - + + ## Things to note in the release changelog: - - - + + -## **Discord contact info** - +## Discord contact info + + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000000..adb6eab67e --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,143 @@ +# Contributing to pokeemerald-expansion + +First off, thanks for helping improve `pokeemerald-expansion`! ❤️ + +All contributions are encouraged and valued. Please make sure to read the relevant section before making your contribution! It will make it a lot easier for you and the maintainers. We're excited to see your contributions. 🎉 + +## Bug Reports + +We use [GitHub](https://github.com/rh-hideout/pokeemerald-expansion/issues?q=sort%3Aupdated-desc+is%3Aissue+is%3Aopen+label%3Abug) issues to track bugs. + +### What should I do before making a bug report? + +- Does your bug occur on the latest unmodified (clean) version of the [`upcoming`](https://github.com/rh-hideout/pokeemerald-expansion/tree/upcoming) or [`master`](https://github.com/rh-hideout/pokeemerald-expansion/tree/master) branch? If not, please do not submit a report - the issue is most likely one introduced by your game. +- Has somebody else already found this issue? This is best done by searching the [bug tracker](https://github.com/rh-hideout/pokeemerald-expansion/issues?q=label%3Abug) to see if anybody else reported it. If there is already an issue, replying to the exsting issue with more information can help solve the problem. + +### How do I submit a bug report? + +If you run into an issue with the project, open an [issue](https://github.com/rh-hideout/pokeemerald-expansion/issues/new). + +The best bug reports have enough information that we won't have to contact you for more information. We welcome all efforts to improve pokeemerald-expansion, but would be very grateful if you completed as much of the checklist as possible in your bug report. This will help other contributiors fix your issue. + +### What happens after I submit a bug report? + +- A maintainer will [label](https://github.com/rh-hideout/pokeemerald-expansion/labels) the bug report. +- A maintainer will try to reproduce the bug with your provided steps. + - If there are no reproduction steps or no obvious way to reproduce the issue, somebody will ask you for those steps. Until the bug can be reproduced, the bug will retain the `bug:unconfirmed` label. Unconfirmed bugs are less likely get fixed. +- If the team is able to reproduce the issue, it will be labeled `bug:confirmed`, and the issue will be left to be [implemented by someone](#Pull-Requests). + - If the issue is particularly game-breaking, a maintainer will add it to a future version's [milestone](), meaning that version will not be released until the problem is solved. + +## Feature Requests + +This section guides you through submitting a feature request for pokeemerald-expansion, **including completely new features and minor improvements to existing functionality**. Following these guidelines will help maintainers and the community to understand your suggestion and find related suggestions. + +- We use [GitHub](https://github.com/rh-hideout/pokeemerald-expansion/issues?q=sort%3Aupdated-desc+is%3Aissue+is%3Aopen+label%3Afeature-request) issues to track feature requests. + +### What should I do before making a feature request? + +- Make sure your request is in [pokeemerald-expansion's scope](team_procedures/scope.md) - if it is not clear if something is in scope, you can start a discussion thread in the [#pr-discussions](https://discord.com/channels/419213663107416084/1102784418369785948) channel of the [the RHH Discord Server](https://discord.gg/6CzjAG6GZk). + +### What should I do before making a feature request? + +- Read the [documentation](https://rh-hideout.github.io/pokeemerald-expansion/) to find out if the functionality is already covered, maybe by an individual configuration. +- Perform a [search](https://github.com/rh-hideout/pokeemerald-expansion/issues) to see if the feature has already been requested. If it has, add a comment to the existing issue instead of opening a new one. + +### How do I submit a feature request? + +To request a feature to be added to the project, open a [feature request](https://github.com/rh-hideout/pokeemerald-expansion/issues/new). + +### What happens after I submit a feature request? + +- A maintainer will [label](https://github.com/rh-hideout/pokeemerald-expansion/labels) the issue. +- If the feature request is out of [scope](team_procedures/scope.md), it will be closed. +- if the request is in scope, any other contributor can volunteer to [fufill it via a pull request](#Pull-Requests). When the request is filled, the request will be closed. + +## Pull Requests + +If you have read all of this and still need help, feel free to start a thread in #pr-discussions of the Discord server or ask questions in #expansion-dev. + +### What should I do before starting a pull request? + +- If you're new to git and GitHub, [Team Aqua's Asset Repo](https://github.com/Pawkkie/Team-Aquas-Asset-Repo/) has a [guide on forking and cloning the repository](https://github.com/Pawkkie/Team-Aquas-Asset-Repo/wiki/The-Basics-of-GitHub). Make sure you have a [local copy](INSTALL.md) of `pokeemerald-expansion`. +- Make sure your contribution is in [scope](team_procedures/scope.md) - if it is not clear if something is in scope, you can start a discussion thread in the [#pr-discussions](https://discord.com/channels/419213663107416084/1102784418369785948) channel of the [the RHH Discord Server!](https://discord.gg/6CzjAG6GZk). +- Choose a branch to contribute your PR to: + - **`master`**: Fixes for bugs that are currently present in the `master` branch. + - **`upcoming`**: All other pull requests. +- Create a new branch from the most recent version of the branch you've chosen. +- If your contribution introduces, removes, or changes a lot of existing code, we reccomend getting a maintainer to agree to review it before you start on the work! We have a table that lists all [current maintainers and their areas of expertise](#maintainers). + +### How do I submit a pull request? + +#### 1. Get a working local copy +If you haven't already, follow [INSTALL.md](INSTALL.md) to get a working local copy of `pokeemerald-expansion`. + +#### 2. Set RHH as a remote +This will designate the main `pokeemerald-expansion` repository as a remote. +```bash +git remote add RHH https://github.com/rh-hideout/pokeemerald-expansion # You can replace RHH with anything you want. This tutorial assumes you used RHH. +``` + +#### 3. Create a new branch +This will create a new branch and switch to it. +```bash +git switch -c newFeature # the name newFeature can be anything you want. This tutorial assumes you used newFeature. +``` + +#### 4. Copy your target branch to your new branch +This will change your new branch to match the latest version of your chosen target branch. +```bash +git reset --hard RHH/upcoming # If your PR is going to target master, replace upcoming with master. +``` + +#### 5. Implement your code +All of your work should go on this new, clean branch. If you already started work on a different branch, you can [cherry-pick](https://git-scm.com/docs/git-cherry-pick) you old commits onto this new branch, or just copy and paste the changes from the original files. + +##### Popular Features / Feature Branches + +If you are implementing functionality from a known community feature branch, it is **strongly** reccomended that you open a discussion thread _before_ starting. There are some situations where maintainers would ask you to use the existing feature branch as a base, and others where maintainers would want a feature to be written from scratch. + +This changes on a case by case basis. + +#### 6. Push your changes +When you push your first commit, you'll need to push the new branch to the remote repo. +```bash +git push --set-upstream origin newFeature +``` + +#### 7. Open Pull Request +Once your work is complete and pushed to the branch on Github, you can open a [pull request from your branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork), targeting the branch you've chosen from `pokeemerald-expansion`. Please fill out the pull request description as completely as possible. + +### What happens after I submit a pull request? + +A maintainer will then assign themselves as a reviewer of your pull request, and may provide feedback in the form of a PR review. + +Contributors are responsible for responding to and updating their branch by addressing the feedback in the review. Contributors are also responsible for making sure the branch passes the checklist at all times. + +Once a maintainer has begun reviewing your PR, **please** do not force-push new changes - normal pushes are fine. Do not worry about git history - we squash most incoming changes. + +Maintainers will measure the submitted pull request against a [merge checklist](docs/team_procedures/merge_checklist.md). + +Once all items on the merge checklist are true, the branch will be merged in. + + +## Maintainers + +This list was last updated 2025 Feb 27. + +| Name | Discord | Currently Active | Areas of Expertise | +| --- | --- | --- | --- | +| [Alex](https://github.com/AlexOn1ine) | rainonline | ✅ | Battle Engine, Battle AI +| [Egg](https://github.com/DizzyEggg) | egg9255 | ✅ | Battle Engine, Battle AI +| [ghoulslash](https://github.com/ghoulslash) | ghoulslash | ✅ | Dexnav, Overworld, Battle Engine +| [Jasper](https://github.com/Bassoonian) | bassoonian | ✅ | Berries, Day / Night System, Followers, Feature Branches +| [MGriffin](https://github.com/mrgriffin) | mgriffin | ✅ | Tests, Trainer Control +| [psf](https://github.com/pkmnsnfrn) | pkmnsnfrn | ✅ | Rematches, Difficulty, Trainer Slides, Fake RTC, Fishing Minigames, Imperial / Metric, OW Item Balls, Sky Battles +| [Hedara](https://github.com/hedara90) | hedara | ✅ | Compression, Sprites +| [Pawkkie](https://github.com/Pawkkie) | pawkkie | ✅ | Battle AI +| [SBird](https://github.com/SBird1337) | karathan | ✅ | Dynamic Multichoice, Damage Calculation, Animations, Trainer Control, Tests +| [AsparagusEduardo](https://github.com/AsparagusEduardo) | asparaguseduardo | Inactive | +| [Agustin](https://github.com/AgustinGDLV) | agustingdlv | Inactive | Gimmicks, Battle Engine, Tests, Items +| [tertu](https://github.com/tertu-m) | tertu | Inactive | Randomizer + +## Attribution +This guide is based on the [contributing.md](https://contributing.md/generator)! diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md new file mode 100644 index 0000000000..81ddb505e9 --- /dev/null +++ b/docs/CONTRIBUTING.md @@ -0,0 +1 @@ +{{#include ../CONTRIBUTING.md}} diff --git a/docs/CREDITS.md b/docs/CREDITS.md new file mode 100644 index 0000000000..23aced71d9 --- /dev/null +++ b/docs/CREDITS.md @@ -0,0 +1 @@ +{{#include ../CREDITS.md}} diff --git a/docs/STYLEGUIDE.md b/docs/STYLEGUIDE.md new file mode 100644 index 0000000000..50351efe5b --- /dev/null +++ b/docs/STYLEGUIDE.md @@ -0,0 +1,399 @@ +# Styleguide and Principles + +## Naming Conventions + +Function names and struct names should be formatted in `PascalCase`. + +```c +void ThisIsCorrect(void); + +struct MyStruct +{ + u8 firstField; + u16 secondField; + ... +}; +``` + +Variables and struct fields should be formatted in `camelCase`. + +```c +int thisIsCorrect = 0; +``` + +Global variables should be prefixed with `g`, and static variables should be +prefixed with `s`. + +```c +extern s32 gMyGlobalVariable; + +static u8 sMyStaticVariable = 0; +``` + +Macros and constants should use `CAPS_WITH_UNDERSCORES`. + +```c +#define MAX_LEVEL 100 + +enum +{ + COLOR_RED, + COLOR_BLUE, + COLOR_GREEN, +}; + +#define ADD_FIVE(x) ((x) + 5) +``` + +## Coding Style + +### Comments + +Ideally, contributions have descriptive variable, function and constant names so as to explain functionality without comments. When a comment is used, the content of the comment should explain _WHY_ a specific system or component works the way it does. + +When describing a system/component in-depth, use block comment syntax. + +```c +/* + * This is an in-depth description of the save block format. Its format is as follows: + * + * Sectors 0 - 13: Save Slot 1 + * Sectors 14 - 27: Save Slot 2 + * ... + */ +``` + +When briefly describing a function or block of code, use a single-line comments +placed on its own line. +There should be a single space directly to the right of `//`. + +```c +// This is supplemental information for the function. If there is a bunch of info, it should +// carry on to the next line. +void ProcessSingleTask(void) +{ + // Short comment describing some noteworthy aspect of the code immediately following. + ... + // Comments should be capitalized and end in a period. +} +``` + +When tagging a data structure that corresponds to an `enum` or some noteworthy +value, place the comment on the same line as the code. +```c +const u8 gPlantlikeMons[] = +{ + FALSE, // SPECIES_BULBASAUR + FALSE, // SPECIES_IVYSAUR + TRUE, // SPECIES_VENUSAUR + FALSE, // SPECIES_CHARMANDER + ... +}; +``` + +### Whitespace + +All `.c` and `.h` files should use 4 spaces--not tabs. +Assembler files (`.s)` use tabs. +Script files (`.inc)` use tabs. + +### Operators + +Assignments and comparison operators should have one space on both sides of `=`. + +```c +int i = 0; // correct +int i=0; // incorrect + +a > b // correct +a>b // incorrect +``` + +The incrementor and decrementor operators should NOT have a space. + +```c +i++; // correct +i ++; // incorrect +``` + +A control statement should have a space between them and their expressions, and the opening bracket should be on the next line. + +```c +for (...) +{ + // correct +} + +for(...) { + // incorrect +} +``` + +A `switch` statement's cases should left-align with the `switch`'s block. + +```c +switch (foo) +{ +case 0: // correct + ... + break; +} + +switch (foo) +{ + case 0: // incorrect + ... + break; +} +``` + +A single empty line should follow a block. + +```c +int MyFunction(int bar) +{ + int foo = 0; + if (bar) + foo++; + + return foo; // correct +} + +int MyFunction(int bar) +{ + int foo = 0; + if (bar) + foo++; + return foo; // incorrect +} +``` + +A chain of `if-else` statements in which any block is more than one line of +code should use braces. If all blocks are single-line, then no braces are necessary. + +```c +if (foo) // correct +{ + return 1; +} +else +{ + MyFunction(); + return 0; +} + +if (foo) // incorrect + return 1; +else +{ + MyFunction(); + return 0; +} +``` + +### Control Structures + +When comparing whether or not a value equals `0`, don't be explicit unless the +situation calls for it. + +```c +if (runTasks) // correct + RunTasks(); + +if (runTasks != 0) // incorrect + RunTasks(); + +if (!PlayerIsOutside()) // correct + RemoveSunglasses(); + +if (PlayerIsOutside() == 0) // incorrect + RemoveSunglasses(); +``` + +When writing a `for` or `while` loop with no body, use a semicolon `;` on the +same line, rather than empty braces. + +```c +for (i = 0; gParty[i].species != SPECIES_NONE; i++); // correct + +for (i = 0; gParty[i].species != SPECIES_NONE; i++) // incorrect +{ } +``` +### Inline Configs + +When adding functionality that is controlled by a config, defines should be checked within the normal control flow of the function unless a data structure requires a change at runtime. +```c +void SetCurrentDifficultyLevel(enum DifficultyLevel desiredDifficulty) +{ +#ifdef B_VAR_DIFFICULTY + return; // Incorrect +#endif + + if (desiredDifficulty > DIFFICULTY_MAX) + desiredDifficulty = DIFFICULTY_MAX; + + VarSet(B_VAR_DIFFICULTY, desiredDifficulty); +} +``` +```c +void SetCurrentDifficultyLevel(enum DifficultyLevel desiredDifficulty) +{ + if (!B_VAR_DIFFICULTY) // Correct + return; + + if (desiredDifficulty > DIFFICULTY_MAX) + desiredDifficulty = DIFFICULTY_MAX; + + VarSet(B_VAR_DIFFICULTY, desiredDifficulty); +} +``` +```c + [MOVE_VINE_WHIP] = + { + .name = COMPOUND_STRING("Vine Whip"), + .description = COMPOUND_STRING( + "Strikes the foe with\n" + "slender, whiplike vines."), + #if B_UPDATED_MOVE_DATA >= GEN_6 // Correct + .pp = 25, + #elif B_UPDATED_MOVE_DATA >= GEN_4 + .pp = 15, + #else + .pp = 10, + #endif + .effect = EFFECT_HIT, + .power = B_UPDATED_MOVE_DATA >= GEN_6 ? 45 : 35, + }, +``` +## Data Type Sizes +When a variable number is used, the data type should generally `u32` (unsigned) or `s32` (signed). There are a few exceptions to this rule, such as: +* Values stored in the saveblock should use the smallest data type possible. +* `EWRAM` variables should use the smallest data type possible. +* Global variables / global struct members use the smallest data type possible. + +## Constants, Enums and Type Checking +Avoid using magic numbers when possible - constants help to make clear why a specific value is used. + +```c +// Incorrect + if (gimmick == 5 && mon->teraType != 0) + return TRUE; + if (gimmick == 4 && mon->shouldUseDynamax) + return TRUE; +``` + +```c +// Correct +#define TYPE_NONE 0 +#define GIMMICK_DYNAMAX 4 +#define GIMMICK_TERA 5 + + if (gimmick == GIMMICK_TERA && mon->teraType != TYPE_NONE) + return TRUE; + if (gimmick == GIMMICK_DYNAMAX && mon->shouldUseDynamax) + return TRUE; +``` + +When several numbers in sequence are used AND those values are not utilized in the saveblock, an enum is used instead. + +```c +//Correct +enum Gimmick +{ + GIMMICK_NONE, + GIMMICK_MEGA, + GIMMICK_ULTRA_BURST, + GIMMICK_Z_MOVE, + GIMMICK_DYNAMAX, + GIMMICK_TERA, + GIMMICKS_COUNT, +}; + + if (gimmick == GIMMICK_TERA && mon->teraType != TYPE_NONE) + return TRUE; + if (gimmick == GIMMICK_DYNAMAX && mon->shouldUseDynamax) + return TRUE; +``` + +When an enum is used, the enum type is used instead of a regular number type to prevent incorrectly set values. + +```c +// Incorrect +bool32 CanActivateGimmick(u32 battler, u32 gimmick) +{ + return gGimmicksInfo[gimmick].CanActivate != NULL && gGimmicksInfo[gimmick].CanActivate(battler); +} + +u32 GetCurrentDifficultyLevel(void) +{ + if (!B_VAR_DIFFICULTY) + return DIFFICULTY_NORMAL; + + return VarGet(B_VAR_DIFFICULTY); +} +``` + +```c +//Correct + +bool32 CanActivateGimmick(u32 battler, enum Gimmick gimmick) +{ + return gGimmicksInfo[gimmick].CanActivate != NULL && gGimmicksInfo[gimmick].CanActivate(battler); +} + +enum DifficultyLevel GetCurrentDifficultyLevel(void) +{ + if (!B_VAR_DIFFICULTY) + return DIFFICULTY_NORMAL; + + return VarGet(B_VAR_DIFFICULTY); +} +``` + +## Principles + +### Minimally Invasive + +New functionality must be as minimally invasive to existing files as possible. When a large amount of new code is introduced, it is best to isolate it in its own file. + +The [`B_VAR_DIFFICULTY`](https://patch-diff.githubusercontent.com/raw/rh-hideout/pokeemerald-expansion/pull/5337.diff) pull request is a good example of lots of new code being introduced in minimally invasive ways. + +### `UNUSED` + +If a function or data is introduced but is never called, it is designated as `UNUSED`. `UNUSED` functions should not be introduced unless neccesary. + +```c +static void UNUSED PadString(const u8 *src, u8 *dst) +{ + u32 i; + + for (i = 0; i < 17 && src[i] != EOS; i++) + dst[i] = src[i]; + + for (; i < 17; i++) + dst[i] = CHAR_SPACE; + + dst[i] = EOS; +} +``` + +### Config Philosophy + +If a branch can modifies saves, the functionality that does so must be gated behind a config, and off by default. + +If a branch has a config that performs either of the following, it should be on by default: +* improves the backend / developer quality of life +* emulates present day, modern day Pokémon + +If a branch's behavior is one that Game Freak does not have a consistent stance on, the default behavior of the config should be disussed by the maintainers. + +All other configs should be off. + +### Save Philosophy + +Until [save migration](https://discord.com/channels/419213663107416084/1108733346864963746) is implemented, branches will only merged in if they do not forcefully break existing game saves. + +When `pokemeerald-expansion` gets to a point where new functionality will require that we break saves, we will merge as many [save-breaking features](https://discord.com/channels/419213663107416084/1202774957776441427) together as possible, and increment the major version number of the project. + +# Attribution +* The majority of the styleguide was written by [garakmon](https://github.com/garakmon) as part of their [PR to pokefirered](). diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 9a921c9373..eecd9a0ec5 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -5,6 +5,8 @@ - [Setting up WSL1 (Legacy Portion)](./legacy_WSL1_INSTALL.md) - [Run documentation site locally](local_mdbook/index.md) - [Ubuntu WSL1/WSL2](local_mdbook/ubuntu_WSL.md) +- [Contributing](./CONTRIBUTING.md) +- [Credits](./CREDITS.md) - [Tutorials]() - [What are AI Flags?](tutorials/ai_flags.md) - [How to add new AI Flags](tutorials/ai_logic.md) diff --git a/docs/team_procedures/merge_checklist.md b/docs/team_procedures/merge_checklist.md new file mode 100644 index 0000000000..8af4247411 --- /dev/null +++ b/docs/team_procedures/merge_checklist.md @@ -0,0 +1,66 @@ +# Document Purpose + +This document is a guide for maintainers to account for all the reccomended steps before merging in a pull request. + + + +# Checklist + +## Is the branch's theoretical functionality in scope? +If you're not sure if a branch's functionality is [in scope](docs/team_procedures/scope.md), start a conversation on Discord to resolve. + +## Does the branch successfully compile? +From `make clean`, the branch should locally compile. + +## Do all CI tests pass? +Contributors are asked to make sure tests pass locally, but maintainers should at least wait for the CI to pass before merging. + +## Have you verified that the functionality works in game without any problems? +If functionality cannot be verified with an automated test, proof of an in game test is required. Do not be afraid to reach out to the contributor or the community to make sure something works in game as it should. + +## If the branch ports behavior from another Pokémon game, have you verified that the behavior functions as faithfully as possible? +We have always tried to make sure we can mimic the original functionality as closely as possible so as to avoid confusion with users and players. Do not be afraid to ask the contributor / community for proof if you cannot personally verify. + +## If the branch is a popular feature within the community with an established feature branch, is this using that established branch as a base? +There are situations where this should and should not happen, and should be discussed with maintainers on a case by case basis. + +## If this branch changes a function that is expected to be modified by users, is there a migration script? +Not everything needs a migration script - if you're unsure, start a discussion. + +## Should new functionality introduced by this branch be gated behind a config? +We don't have a strict definition of when configs should be used, but you can start with + +> Why SHOULDN'T this be a config? + +## Are tests written for everything that can be tested? +If you're not sure if something CAN be tested, start a discussion. Some contributors may not be capable of writing tests - we should guide them in #expansion-tests to do so. + +If any new tests are `KNOWN_FAILING`, issues should be opened describing each of the `KNOWN_FAILING` tests and our understanding of why they fail. + +## Does the branch meet our [config philosophy](docs/styleguide#config-philosophy)? + +## Does the branch meet our [saves philosophy](docs/styleguide#saves-philosophy)? + +## Does the submitted code follow the [styleguide](docs/styleguide)? +This applies to code that comes from other branches or games. + +## Is the pull request appropriately labeled? +Without labels, the CHANGELOG will not be properly formatted. For specifically the `bugfix` label, an additional label, detailing what area the bug exists in is required. + +## Is `pokeemerald-expansion` free from a merge freeze? +Our [release schedule](docs/team_procedures/schedule.md) prevents us from merging Big Features and non-bugfixes within certain dates close to a release. Please use `/release` in the RHH Discord to clarify when these are occuring. + +# Merging + +When a feature has passed all of the items on the checklist, it is ready to be merged. From GitHub's interface, there are three different options for merging: + +## Squash and merge +This should be used for all PRs _except_ when merging from either: +* a publicly available feature branch from by the community OR +* `upcoming`, `master` or `pret/pokeemerald`. + +## Create a merge commit +When the branch uses a publicly available feature branch from by the community, use "Create a merge commit" to preserve history for users. + +## Rebase and merge +We do not use this ever.