Update contributing guidelines

CONTRIBUTING.md

@@ -4,18 +4,23 @@ Thank you for your interest in contributing! We welcome pull requests and docume
 
 If you have any questions, feel free to join our **[Discord]** server.
 
+
+# A Reminder
+
+Super Mario Galaxy's codebase was directly built upon for Super Mario Galaxy 2. As such, a **lot** of code will end up being ported over to **[Garigari]**. We would appreciate ensuring Petari code adheres to the **current** contributing guidelines, as they may have changed from when the file was originally submitted. This will help ensure both projects are maintained properly.
+
 ---
 
 # Before Submitting
 
 Before opening a pull request, make sure your changes satisfy all of the following:
 
-- Run `clang-format` on every modified file. **Formatting is required.**
+- Run `clang-format` on every modified file. **Formatting is required.**. If using VSCode, please enable auto-formatting on save.
 - Run `ninja` and ensure the object you're working on is **1:1 matching**. Non-matching objects cannot be marked as matching and will break the link step.
 - Run `objdiff` to verify every function still matches the original binary.
 - Once all modified functions are marked as decompiled, run `ninja` again to verify it can successfully link.
 - Keep your pull request focused on **a single object whenever possible**. Multiple objects are acceptable only when they are directly related or depend on each other.
-- Every non-matching function in your pull request **must** have a decomp.me scratch linked in a comment directly above the function.
+- Every non-matching function in your pull request **must** have a `decomp.me` scratch linked in a comment directly above the function.
 
 ---
 
@@ -50,8 +55,11 @@ Contributors should be familiar with:
 - Use `nullptr` instead of `0` for pointers in C++.
 - Use `NULL` for pointers in C.
 - Use `0` only for numeric values.
-- Use the `f` suffix for floating point literals.
-- Avoid humorous comments, jokes, or commentary about typos, compiler behavior, or reverse engineering discoveries. Comments should remain professional and describe the code, not the contributor's thought process.
+- Pointer values should **explicitly** compare to `nullptr`.
+- Use decimal values instead of hex for non-enum numeric values.
+- Use enum values instead of magic numbers.
+- Use the `f` suffix for floating point literals, and include the trailing `.0`. (eg: `123.0f`)
+- Avoid comments that do not provide insightful information. Comments should be clear and concise, and be relevant to the code, or its context within or between Super Mario Galaxy games.
 
 ## Headers
 
@@ -65,24 +73,24 @@ Prefer forward declarations whenever possible instead of adding unnecessary incl
 
 ## Includes
 
-Use angle brackets (`<>`) for library and SDK headers:
-
-```cpp
-#include <revolution/os.h>
-#include <JSystem/JGeometry.hpp>
-```
-
 Use quotes (`""`) for project headers:
 
 ```cpp
 #include "Game/MapObj/HipDropRock.hpp"
 ```
 
-Project includes should always be relative to the `include` directory.
+Use angle brackets (`<>`) for library and SDK headers:
+
+```cpp
+#include <revolution/OS.h>
+#include <JSystem/JGeometry.hpp>
+```
+
+Project includes should always be relative to the `include` directory. Project headers should always be included before library headers. 
 
 ## Classes
 
-Do not use `this->` unless it is required for compilation.
+Do **not** use `this->` unless it is required for compilation.
 
 Member functions should appear in the following order:
 
@@ -91,8 +99,12 @@ Member functions should appear in the following order:
 3. Operators
 4. Virtual functions
 5. Other member functions
+6. Nerve executor functions
+7. Inlines
 
-Virtual functions **must** appear in the same order as the original vtable. Preserving the original virtual function order takes precedence over the ordering above.
+Virtual functions **must** appear in the same order as the original vtable. Preserving the original virtual function order takes precedence over the ordering above. The only exception is for destructors, where unless required for matching, can be omitted in favor of the implicit compiler-generated destructor. Non-virtual functions in header should match appearance of functions in source where applicable.
+
+Class inheritance and class members should be left fully public unless otherwise indicated.
 
 Class members should be documented using the project's standard format:
 
@@ -100,6 +112,8 @@ Class members should be documented using the project's standard format:
 /* 0x10 */ s32 mValue;    ///< Description.
 ```
 
+Do **not** manually add padding between members unless it is required for matching. The compiler will automatically pack and pad types smaller than 4 bytes. If a member variable goes unused, mark it with a comment indicating so, (eg: `///< Unused`)
+
 Functions should also use the project's Doxygen style:
 
 ```cpp
@@ -111,83 +125,237 @@ bool doThing(int* pValue);
 
 If you encounter an older header that has not yet been documented, documentation improvements are always appreciated.
 
+## Constructors
+
+Class constructors must use initializer lists where applicable rather than initializing members directly inside the function scope. Members that are initialized to default values should be included with empty initializers rather than explicitly initializing with a default value.
+
+```cpp
+AstroDomeSky::AstroDomeSky(const char* pName) : LiveActor(pName), mBrkFrame(), mColor(0x00, 0x00, 0x00, 0xFF), mProjmap() {
+}
+```
+
+## Enums
+
+Enums must include a valid `PascalCase` type name, unless otherwise indicated by official names, and enum values must include the type name prefixing the value name, and a comment detailing the raw enum value.
+
+```cpp
+enum SwitchType {
+    /* 0x0 */ SwitchType_QuarterPlus,
+    /* 0x1 */ SwitchType_QuarterMinus,
+    /* 0x2 */ SwitchType_HalfPlus,
+    /* 0x3 */ SwitchType_HalfMinus,
+};
+```
+
+Be sure to check relevant cases for common enums and where they are used. Commonly used enums are in `SceneFunction.hpp`, `ActorSensorUtil.hpp`, `AudSoundId.hpp`, and many others, so be sure to become acquainted with where enums are common.
+
+## Namespaces
+
+Functions belonging to a namespace should be explicit when referred to in source files, and individually labelled rather than grouped into a namespace block. 
+
+```cpp
+f32 MR::getInterpolateValue(f32 t, f32 start, f32 end) {
+    return start + (end - start) * t;
+}
+
+f32 MR::getLinerValue(f32 x, f32 start, f32 end, f32 max) {
+    return MR::getInterpolateValue(x / max, start, end);
+}
+```
+
+The only exceptions to this are Nerve definitions the anonymous namespace, which should be grouped in one block unless symbol order indicates otherwise.
+
+```cpp
+namespace {
+    static const s32 sFreeBloomFlyUpStep = 15;
+    static const f32 sSpeedWaitAir = 10.0f;
+    ...
+    static const s32 sWindRemainTime = 30;
+    static const f32 sJumpSpeedY = 18.0f;
+};  // namespace
+```
+
+References to elements of the anonymous namespace **must** include the `::anonymousScopeResolutionOperator` prefix. Furthermore, namespace blocks must end with a `;` to align the namespace comment. Please include the namespace comment manually if it is not automatically added.
+
+Certain groups of namespaces are potentially static classes. Whether they are a namespace or a class should be decided on a case-by-case basis and should be discussed with the rest of the decomp team if you are creating a new one.
+
+## Nerves
+
+Empty/simple nerve executors should be explicitly written in source rather than header, and never marked as `inline`.
+
+Nerve declarations normally occur in source, not in the headers, and typically stick to a standard layout of being defined in a namespace block.
+
+```cpp
+namespace NrvTreasureSpot {
+    NEW_NERVE(TreasureSpotNrvWait, TreasureSpot, Wait);
+    NEW_NERVE(TreasureSpotNrvEnd, TreasureSpot, End);
+    NEW_NERVE(TreasureSpotNrvSpout, TreasureSpot, Spout);
+};  // namespace NrvTreasureSpot
+```
+
+When getting a nerve, please use the relevant macros.
+
+```cpp
+&NrvTreasureSpot::TreasureSpotNrvSpout::sInstance; // No
+GET_NERVE(TreasureSpot, TreasureSpotNrvSpout);     // Yes
+```
+
 ## Naming
 
-Known symbols must match the original names exactly, including spelling mistakes.
+Known symbols and enum names must match the original names exactly, including spelling mistakes. Where applicable, the Korean debug symbol consts for `RMGK01` from the Nvidia Shield release for SMG1 should still be used / applied if missing.
 
 Do **not** "fix" typos in symbol names.
 
 Use the following naming conventions:
 
 - Member variables begin with `m`
-- Pointer parameters begin with `p`
-- Reference parameters begin with `r`
+- Pointer variables begin with `p`
+- Reference variables begin with `r`
 - Unknown static variables begin with `s`
 - Unknown globals begin with `g`
 - Unknown functions use `camelCase`
 
+For unknown functions or inlines, follow the known verbiage of the official symbols.
+
+For basis vectors, short directional names are preferred over verbose or axis-based names unless readability is a concern (eg `mFront` over `mFrontVec`, `up` over `upVec`, `side` over `xDir`). Furthermore, the X, Y, and Z directions indicate Side, Up, and Front directions. 
+
 ## Types
 
 If an original function uses `int`, then use `int`.
 
 Do **not** replace it with `s32`, even if they are the same size, as they produce different mangled symbols.
 
+Typedefs should be used in place of their raw type counterparts. For example: 
+```cpp
+void func(const JGeometry::TVec3<float> &, float *, unsigned long)
+```
+should be written instead as
+```cpp
+void func(const TVec3&, f32*, u32)
+```
+
+Unless logically necessary, or code patterns suggest otherwise, `JGeometry` types are preferred over `RVL` types (eg `TVec3f` over `Vec`, `TQuat4f` over `Quaternion`, `TPos3f` over `MtxPtr`, `TProj3f*` over `Mtx44Ptr`) for member and local variables. Function parameters should follow nearby code standards, and **always** adhere to the types indicated by Petari where applicable.
+
+Because of implicit casting, explicit casts should be avoided unless necessary for `JGeometry` types passed as `RVL` types.
+
+## Function Arguments
+
+Many commonly-used functions have default arguments that will be applied if omitted. Do not include the raw values of these default arguments when calling functions.
+
+```cpp
+MR::isSameDirection(front, up, 0.01f); // No, since the default value is 0.01f
+MR::isSameDirection(front, up);        // Yes, if default value is used
+MR::isSameDirection(front, up, 0.1f);  // Yes, if non-default value is used
+```
+
+Many functions relating to the Wiimote will pass an argument indicating which Wiimote (or pad) channel is in use. Please use the relevant `WPAD` defines rather than the raw number for the pad channel (eg: `WPAD_CHAN0` for pad `0`/P1's Wiimote).
+
+## Regional Differences
+
+Regional differences in code should appear in order of regional release, and be contained in preprocessor blocks explicitly indicating the versions that include that code. Please keep revisional difference blocks as small and self-contained as possible.
+
+```cpp
+void LuigiMailDirector::lost() {
+#if (VERSION == RMGJ01 || VERSION == RMGE01 || VERSION == RMGP01)
+    mLostMessageNum++;
+#elif (VERSION == RMGK01)
+    mLostMessageNum = 1;
+    mFoundMessageNum = 0;
+#endif
+}
+```
+
+Please also include a `// REGIONAL` comment nearby to indicate regional differences.
+
 ---
 
-# Non-Matching Code
+# Formatting
+
+Code with directly similar purpose or logical functioning should be grouped together without extra newlines in between. However, an extra newline must be included after the end of closing brace of a scope block and any new code immediately after.
+
+```cpp
+if (isNerve(GET_NERVE(Coin, CoinNrvNonActive))) {
+    MR::invalidateClipping(this);
+    MR::validateHitSensors(this);
+    MR::onBind(this);
+
+    MR::calcGravityOrZero(this);
+    mVelocity.set(-mGravity * 30.0f);
+
+    setNerve(GET_NERVE(Coin, CoinNrvMove));
+
+    return true;
+}
+
+return false;
+```
+
+Expressions should favor readability when it does not impact matching. Long single-lines with multiple inlined expressions should be broken into multiple lines, however chained function calls should remain as one line.
+
+---
+
+# Non-Matching, Fakematching, Unclean Code, TODOs
+
+Please mark relevant incomplete or incorrect code that needs to be updated with the relevant comment indicator style.
+
+## Non-Matching
 
 Non-matching code is welcome, but it should be easy for others to continue working on.
 
 For every non-matching function:
 
-- Explain why the function does not currently match, if known.
+- Explain why the function does not currently match, if known, in a comment marked `FIXME`.
 - Include a link to the corresponding decomp.me scratch directly above the function.
 - Update the comment if the scratch changes or a better reproduction is found.
 
 For example:
 
 ```cpp
-// NON_MATCHING
-// decomp.me: https://decomp.me/scratch/XXXXX
-// Stack usage due to inlines not matching
-
 void someFunction() {
+    // FIXME: Stack usage due to inlines not matching
+    // https://decomp.me/scratch/XXXXX
     ...
 }
 ```
 
 If you do not know why the function does not match, simply include the decomp.me link and omit the explanation.
 
+## Fakematching 
+
+Code that outputs the same byte-for-byte output when compiling, but is not how the code would have been written are considered "Fakematches". Avoid submitting these wherever possible, and if you *must* submit a fakematch or come across one in the repository, please mark it with a `// FAKEMATCH` comment. 
+
+## Unclean Code
+
+Much of the repository will not be clean due to the nature of a constantly evolving set of standards. Please try to keep code clean as much as possible, and if you come across code that does not meet the contributing standard, please either clean it or mark the code with a `// CLEANME` comment.
+
+## TODOs
+
+If something in the repository requires further investigation that does not fall under the above categories, a `// TODO` comment may also be left, with a **clear and concise** explanation of what needs to be investigated.
+
 ---
 
 # Generated Code
 
-Decompiler output and AI tools can be helpful for understanding code, suggesting variable names, or improving the readability of existing C++ code.
+Decompiler output can be helpful for understanding code.
 
 However:
 
-- **Do not copy and paste IDA output directly into the repository.**
-- **Do not copy and paste AI-generated code directly into the repository.**
+- **Do not copy and paste IDA/Ghidra output directly into the repository.**
+- **Do not submit AI-generated or otherwise directly machine-generated code into the repository.**
 
 Submissions containing obvious decompiler output or AI-generated code will be rejected.
-
-AI is perfectly acceptable for tasks such as:
-
-- Suggesting better variable or function names.
-- Cleaning up existing C++ code.
-- Explaining PowerPC assembly.
-- Answering reverse engineering questions.
-
 Contributors are expected to understand the code they submit and ensure it accurately matches the original.
 
 ---
 
-# Questions
+# Discord
 
-Need help?
+Need help? Want to discuss with the Petari/Garigari decompilation team?
 
-Join our **[Discord]** server and we'll be happy to answer any questions in our decompilation channel!
+Join our Discord server via the **[Luma's Workshop]** website, and we'll be happy to answer any questions in our decompilation channel!
 
 <!----------------------------------------------------------------------------->
 
-[Discord]: https://discord.gg/ZxEqyYeZbf
+[Luma's Workshop]: https://www.lumasworkshop.com/
+[Discord]: #discord
+[Garigari]: https://github.com/SMGCommunity/Garigari

README.md

@@ -29,18 +29,8 @@ This repository does **not** contain any game assets or assembly whatsoever. An
 
 This project is **not** meant to be an effort to create a PC port. Join the Discord server to learn more and ask about our work-in-progress PC port "AstroCore".
 
-## Regarding AI Usage
-
-A lot of discussion and accusations have been made claiming we used AI/LLMs to accelerate the decompilation process. **We did not**.
-
-Although AI was allowed for tasks that did not directly affect progress, such as variable naming, code cleanup, and documentation, **it never ended up playing a role in this project**. The fast progress acceleration of the project was a result of **new collaborators**, a ton of **motivation**, and **great community efforts**. It was not a result of AI usage or any other kind of automated decompilation work.
-
-If you don't trust this statement enough, read the source code yourself and form your own conclusions.
-
-Below are our AI usage guidelines:
-
 > [!NOTE]
-> AI may be used for code cleanup, formatting, documentation, and naming assistance. AI-generated decompilation work is not allowed. Pull requests containing obvious AI-generated decompilation output or other AI slop will be rejected. Contributors should be able to explain and justify any decompilation work they submit. This also applies to all tool-generated code. We want to keep this project as human as possible.
+> This is a community-focused project that would like to remain as human as possible. AI-generated or otherwise automated contributions will be rejected. Contributors should be able to explain and justify any work they submit.
 
 Dependencies
 ============