Skip to content
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

Cleanup formatting with clang-format. #558

Merged
merged 1 commit into from
Nov 22, 2022

Conversation

PhlexPlexico
Copy link
Collaborator

This PR introduces a new workflow on the development and main branches, which will auto-format and commit the code if it fails to meet the specific clang-formatting. This is to ensure we keep code clean, and developers don't have to install clang-format if they choose not to (even though it is highly recommended).

I've also gone ahead and ran the code formatting once as well, hence the large amounts of files changed. I can always back this out if needed as well!

The .clang-format config was taken from zeldaret's MM decomp project as well, to ensure parity between Zelda projects.

I'm also leaving this one in draft until we have most PRs closed, to ensure we don't interrupt anyone's workflows when this goes in because it will surely cause merge conflicts.

@PhlexPlexico PhlexPlexico force-pushed the clang-formatting branch 2 times, most recently from 27f28cd to 1024312 Compare October 5, 2022 18:28
@PhlexPlexico PhlexPlexico marked this pull request as ready for review October 5, 2022 19:02
@Kewlan
Copy link
Collaborator

Kewlan commented Oct 5, 2022

1: I think it should be turned off for (or adjusted for) the following:

  • Settings
  • Item locations
  • Item list
  • Descriptions
  • The seqTypesSFX list in sound_effects.cpp

Maybe for the following:

  • Location access
  • Hints

2: Other suggestions:

  • Remove column limit
  • Allow the previous of this:
    image
  • Add IndentPPDirectives: BeforeHash
  • Maybe add AlignConsecutiveAssignments? It's style seems to be used a lot.

@PhlexPlexico
Copy link
Collaborator Author

Settings

Yeah I can see this one being an issue. Readability isn't that great with clang-format messing around with all of it. I think there may be a way to adjust this without having a bunch of // clang-format off littered around the options. I'll continue to look into it!

Item Location

I'm a bit impartial about this one, personally, mainly due to the fact in it's current state it's not that readable since you have to scroll in order to see the rest of values, on 1920x1080 (or low PPI screens) it's even worse to go through. However, that is something we probably could ignore if this is preferred overall because I can also see the spacing being an issue.

Item List

Same here, a bit impartial about having to scroll, but this one does look a bit cleaner at least/easier to navigate.

Descriptions

Ah yes, this one should have an option to keep everything aligned with the text and = sign... I think that file could be completely ignored since it does already have its own styling.

SeqTypesSFX

Hmm, I guess my question for this is why are the comments prepended where all other comments are either above or appended in the array? As an example, see cosmetics.cpp in tunicColors.

Location Access

Again with column limits. I'm sure there can be an in-between solution for this :) even with AlignConsecutiveAssignments it still has some issues!

Hints

If it's mainly the hintSettingTable, so long as they can be kept consistent as well.

Col. Limit

I'm against removing this overall, as I feel this is one of the larger style choices that are broken most often, and this styling is meant to make it consistent across projects. However, I'm willing to part ways with this one as well if that's a general consensus too! 😄

IndentPPDirectives: BeforeHash

LGTM 👍

AlignConsecutiveAssignments

👍

@Kewlan
Copy link
Collaborator

Kewlan commented Oct 5, 2022

SeqTypesSFX

Hmm, I guess my question for this is why are the comments prepended where all other comments are either above or appended in the array? As an example, see cosmetics.cpp in tunicColors.

They just happen to be prepended because I copied the style from Ura's BGM shuffle implementation. (Which btw has a similar list, seqTypesMusic in music.cpp, that should be in the same style.) Whether they're before or after doesn't really matter, it's the alignment I wanna keep. Maybe possible with clang?

@PhlexPlexico
Copy link
Collaborator Author

Hmm, perhaps its possible here, if not we can simply set it to ignore as well, especially if it's just specific lists, too! I'll see what I can do!

@PhlexPlexico PhlexPlexico force-pushed the clang-formatting branch 3 times, most recently from c9228e7 to 67d9add Compare October 11, 2022 02:49
@PhlexPlexico PhlexPlexico force-pushed the clang-formatting branch 4 times, most recently from fb20f17 to f60780d Compare October 17, 2022 14:45
@gamestabled
Copy link
Owner

Is this PR waiting on anything else besides conflict resolution?

@PhlexPlexico
Copy link
Collaborator Author

I don't believe so, I think everything was acknowledged in the discord. I can rebase and fix the conflicts here shortly.

@PhlexPlexico PhlexPlexico force-pushed the clang-formatting branch 4 times, most recently from 44450a3 to d7807fa Compare November 8, 2022 16:03
Bring in .clang-format file from zeldaret/mm.

Include other clang format changes.

Per review adjustments from @Kewlan and @HylianFreddy

Update clang format for short enums.

Update trickNameTable with custom formatting.
@gymnast86 gymnast86 merged commit 152ac39 into gamestabled:main Nov 22, 2022
PhlexPlexico added a commit to PhlexPlexico/OoT3D_Randomizer that referenced this pull request Dec 31, 2022
* Fix flag for Granny's item (gamestabled#575)

* Fix Various Grotto Return Issues (gamestabled#576)

* Add in overworld spawns, warp songs, and owl drops to ER

* Add support for one-way entrances and decoupled entrances in entrance tracker

* Restructure the entrance tracker to properly account for the GV -> Lake Hylia entrance

* Remove debugging statements

* Fix formatting in new logic areas

* Add descriptions for the new settings

* Allow users to choose which entrance types go into the mixed pool

* Add Ganon's Castle entrances to the entranceData table

* Adjust yellow color for color blindness

* Change 'Src' and 'Dst' to 'To' and 'From'

* Add new ER options to full chaos preset

* Change the condition for handling random grottos

* Properly clear grotto state and handle dynamic exit edge case

* add missing include

* Fix child B button glitch (gamestabled#581)

* add decoupled entrances to spoiler log output (gamestabled#580)

* Generalize grotto return fix (gamestabled#578)

* Fix glitched logic errors (gamestabled#582)

* Fix glitchled logic errors near world root

* remove adult rquirement from glitched logic forest gs

* Various logic fixes (gamestabled#583)

* MS shuffle logic fixes

* More logic fixes

* ZD OoB Jumpslash fix

* KF Deku Tree logic fix

* Deku Tree to KF logic fix

* Additional Mirror World settings (gamestabled#585)

* Random mirror world settings

* Make MW option U8

* Complete Italian translations (gamestabled#586)

* Complete Italian translations

Head of squashed commits: 0760138

* Remove obsolete overloads

* Change handling of lengthy shop item names

* Minor changes to English hints

End of squashed commits

* Support plural for item hints

* Add plural markers for Italian

+ minor fixes

* Misc. fixes

* Various Bug Fixes (gamestabled#588)

* Fix overworld spawn at grotto exit

* Fix crash at spoiler log generation

* Fix blue warps leading to grotto exits

* Curse traps fixes

- room/heat timers clearing the curses;
- FW depleting the curse timer;

* Fix description for Child Deadhand w/ Sticks trick

* Prevent picking up chests and Pedestal of Time

* Complete german translation (gamestabled#590)

* Cleanup formatting with clang-format. (gamestabled#558)

Bring in .clang-format file from zeldaret/mm.

Include other clang format changes.

Per review adjustments from @Kewlan and @HylianFreddy

Update clang format for short enums.

Update trickNameTable with custom formatting.

* Run automatic format script as code does not match clang format rules.

Developers please rebase to avoid merge conflicts!

* Fix some formatting errors (gamestabled#596)

* Fix glitchled logic errors near world root

* remove adult rquirement from glitched logic forest gs

* fix some formatting errors

* Run automatic format script as code does not match clang format rules.

Developers please rebase to avoid merge conflicts!

* Re-add locations that went missing from the clang formatter (gamestabled#597)

* Fix glitchled logic errors near world root

* remove adult rquirement from glitched logic forest gs

* fix some formatting errors

* the formatter also deleted some newer locations apparently, lol

* Run automatic format script as code does not match clang format rules.

Developers please rebase to avoid merge conflicts!

* Add boss entryways to area logic (gamestabled#598)

Preparations for boss room shuffle

* Prevent duped decoupled entrances in spoiler log (gamestabled#589)

* prevent duped decoupled entrances in spoiler log

* fix more clang-format merge conflicts

* Even more fixes and tweaks (gamestabled#595)

- Allow From/To toggle in All entrance tracker, fixing spoiling bug.
Also change code in Gfx_DrawButtonPrompts to use offset.
- Add rupees to shop items in location tracker.
- Always reveal LACS item location when Ganon's Boss Key is placed there.
- Add Megaton Hammer to ISG logic.
- Restore settings to default before loading a custom preset, properly resetting new options missing from that preset.
- Add new and adjust existing descriptions for clarity.
- Adjust code for custom info menu options to support optional options.
- Split some location tracker groups for easier scrolling/readability and to match entrance tracker.

* Settings string improvements (gamestabled#599)

These changes allows us to freely move around and indent settings without affecting future custom presets.
Also allows both the setting and the option to have a newline and text after/below.

* Add missing scenes to interior areas (gamestabled#601)

* Fix glitchled logic errors near world root

* remove adult rquirement from glitched logic forest gs

* fix some formatting errors

* the formatter also deleted some newer locations apparently, lol

* Add scenes to interiors where they were missed

* Run automatic format script as code does not match clang format rules.

Developers please rebase to avoid merge conflicts!

* really fix decoupled entrances in spoiler log and preserve playthrough sphere (gamestabled#600)

* Minor clang format fixes (gamestabled#602)

* Fix merge error

* Fix some weird formatting in C code

* Add manual trigger for clang-format

* Add SFX to junk hints (gamestabled#603)

* Add SFX to junk hints

Old implementation: 8b678b6

* Minor fix to Italian hint

* Fixes and tweak 4 (gamestabled#604)

* Fixes and tweak 4
- Make random cosmetics consistent using a separate generator
- Remove patch for brown boulder for multiplayer
- Update the item text of Ruto's pickup message in Jabu

* Improve Ruto pickup message

Co-authored-by: HylianFreddy <[email protected]>

Co-authored-by: HylianFreddy <[email protected]>

* Fix Saria's hints and external texture mods (gamestabled#605)

* Don't touch Link's model if custom tunics are off

* Register Saria's hints after talking to stone

* docs: highlight region restrictions (gamestabled#606)

* Guarantee Goron Pot Reward on First Try (gamestabled#607)

* fix entrance playthrough not showing in the spoiler log for vanilla logic (gamestabled#608)

* Fix excluded locations always receiving the same items (gamestabled#610)

* MS shuffle logic fixes

* More logic fixes

* ZD OoB Jumpslash fix

* KF Deku Tree logic fix

* Deku Tree to KF logic fix

* Shuffle item pool after generation

* Hide Exclude Locations options when certain settings are on (gamestabled#592)

* Properly Exclude Locations

* Indent fix

* Run automatic format script as code does not match clang format rules.

Developers please rebase to avoid merge conflicts!

* Add option to use Rupees as ammo (gamestabled#591)

* Use Rupees as Ammo

* Make magic a bit less expensive

* Allow arrows replenishing when using fire/ice/light bows

* Quick fix, missing parenthesis

* Further fix to magic count

Magic only gets spent when it's not being used, aside from Lens of Truth.

* Cosmetics: Apply custom Navi colors to target reticle (gamestabled#611)

* Rename fairy colors functions

* Fix case typo

* Apply custom Navi colors to target reticle

* Run automatic format script as code does not match clang format rules.

Developers please rebase to avoid merge conflicts!

* Improve build process (gamestabled#614)

* Attempt using a new image.

* Rebase + update versions for actions.

Co-authored-by: HylianFreddy <[email protected]>
Co-authored-by: gymnast86 <[email protected]>
Co-authored-by: Adam Bird <[email protected]>
Co-authored-by: Nessy <[email protected]>
Co-authored-by: Schicksal88 <[email protected]>
Co-authored-by: gymnast86 <[email protected]>
Co-authored-by: Colin E <[email protected]>
Co-authored-by: Jesse Stricker <[email protected]>
Co-authored-by: Charade <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants