FiendsOfTheElements
Preliminary formatting changes
This formats all the code in the repo and (eventually) will check formatting and checking that it can be built in circleci before allowing stuff to be merged in
this looks good to me one question: what's the criteria for keeping lists' items on a single line or putting them on each of their line? going over everything it seems a bit random what list get expanded that way or not
this looks good to me one question: what's the criteria for keeping lists' items on a single line or putting them on each of their line? going over everything it seems a bit random what list get expanded that way or not
If it's anything like prettier it switches to multiline when it doesn't fit within the line length you specify. If we don't explicitly set one somehow it's probably like 120 chars.
In my experience, forced style is worth the price of nobody being 100% happy with what it looks like, as long as IDE integrations can be tweaked to everybody's liking. I've been a format-on-save dude for a few years now, and just not even thinking about pressing enter or spacing things properly while coding, knowing that just saving once makes everything 'perfect' is a cool thing to get used to.
I'd prefer it not do anything the VisualStudio "Format Document" command doesn't.
In the end, you want me to fix my own code as long as i'm around. If you change my code to the point, where i don't recognize it anymore, it'll severly hamper me.
Besides all theese formatting changes don't make the problems of the code go away. Spaghetti code will still be spaghetti code. And shitty code will at most become pretty shitty code.
But all this does, is ensure that neither you nor i know, what my code does. So as long as i'm around, I'd like to keep my code intact.
I'd prefer it not do anything the VisualStudio "Format Document" command doesn't.
This is the result of running dotnet format which is put out by the dotnet team, so while its not baked into the editor, its nearly just as standard. I'll look into making "Format Document" in Visual Studio run the dotnet formatter, or see if there is a separate command in Visual Studio that will do it for us.
In the end, you want me to fix my own code as long as i'm around. If you change my code to the point, where i don't recognize it anymore, it'll severly hamper me.
Besides all theese formatting changes don't make the problems of the code go away. Spaghetti code will still be spaghetti code. And shitty code will at most become pretty shitty code.
But all this does, is ensure that neither you nor i know, what my code does. So as long as i'm around, I'd like to keep my code intact.
This should be a one time migration, afterwards all the errors will show up in your editor and it will suggest how to fix it as well as block merging stuff that doesnt meet the style rules we end up agreeing on. Therefore, for everything after this is merged, it will be the exact code as you had it before that code is merged in.
this looks good to me one question: what's the criteria for keeping lists' items on a single line or putting them on each of their line? going over everything it seems a bit random what list get expanded that way or not
If it's anything like prettier it switches to multiline when it doesn't fit within the line length you specify. If we don't explicitly set one somehow it's probably like 120 chars.
In my experience, forced style is worth the price of nobody being 100% happy with what it looks like, as long as IDE integrations can be tweaked to everybody's liking. I've been a format-on-save dude for a few years now, and just not even thinking about pressing enter or spacing things properly while coding, knowing that just saving once makes everything 'perfect' is a cool thing to get used to.
Unfortunately it appears the dotnet-format does not have prettier-style line breaking, and what we are seeing is the rule to preserve oneline vs when things are on multi-line, it just breaks it all into its own line. There is a github issue opened in mid 2019 which has the most ❤️ reactions and 👍 reactions out of all current issues, so there is demand and we might get that update in the future.
On the topic of line breaking as it currently stands though, we might need to disable the current rules as it does not enforce line length but rather forces you, for each collection, to have everything on one line or everything in mutliple lines, not ideal.
On the topic of line breaking as it currently stands though, we might need to disable the current rules as it does not enforce line length but rather forces you, for each collection, to have everything on one line or everything in mutliple lines, not ideal.
There just seems to be a few lists that aren't on only one line but get to keep their formatting, like in dialog.cs
(new List<MapLocation> { MapLocation.ConeriaCastleRoom1, MapLocation.ConeriaCastleRoom2, MapLocation.MirageTower1, MapLocation.SeaShrine1, MapLocation.SkyPalace1, MapLocation.TempleOfFiends1Room1, MapLocation.TempleOfFiends1Room2, MapLocation.TempleOfFiends1Room3, MapLocation.TempleOfFiends1Room4, MapLocation.CastleOrdeals1, MapLocation.TempleOfFiends2, MapLocation.EarthCave1, MapLocation.GurguVolcano1, MapLocation.IceCave1, MapLocation.MarshCave1 },
looking at it, maybe it's not applying the rule to lists inside lists.
On the topic of line breaking as it currently stands though, we might need to disable the current rules as it does not enforce line length but rather forces you, for each collection, to have everything on one line or everything in mutliple lines, not ideal.
There just seems to be a few lists that aren't on only one line but get to keep their formatting, like in dialog.cs
(new List<MapLocation> { MapLocation.ConeriaCastleRoom1, MapLocation.ConeriaCastleRoom2, MapLocation.MirageTower1, MapLocation.SeaShrine1, MapLocation.SkyPalace1, MapLocation.TempleOfFiends1Room1, MapLocation.TempleOfFiends1Room2, MapLocation.TempleOfFiends1Room3, MapLocation.TempleOfFiends1Room4, MapLocation.CastleOrdeals1, MapLocation.TempleOfFiends2, MapLocation.EarthCave1, MapLocation.GurguVolcano1, MapLocation.IceCave1, MapLocation.MarshCave1 },looking at it, maybe it's not applying the rule to lists inside lists.
that is also entirely possible, Im open to suggestions on how you think we should handle this, try to follow one the the rules or disable them entirely and looking into a third party line length checker, or disable them and let PR reviewers catch that stuff.
I'm still not entirely sure, what this will accomplish. I can see a lot of really badly formatted code in the project yes. But formatting it "nicely" will not make the code that much more readable and maintainable.
I mean, just look at what it did with enemizer. That's now a whooping 4800 lines instead of already unbearable 3750. Can anyone seriously argument, that he now understands what this code does any more than without theese changes? Besides beeing spaghetti code, the current formatting isn't actually bad. It's sure different, but consistent, clear and it conveys the individial constructs. It's just a really long spaghettified code piece that'll take a long time to get into, no matter what formatting.
Or take OverWorldMap. The changes it does, are superficial. A few extra lines here and there. But the algorithm of F/E Shuffle still has the same complexity.
Also the Formatter doesn't change bad variable and function names. It doesn't change that half our project is a partial class.
I will find theese formatting restrictions mostly annoying, especially if they prevent merges and commits. But they will neither make the code more readable nor more maintainable. I'm 100% sure of that.
This is temporarily on hold as I work on the FFRBot rewrite, but seeing as how most of the team wants standardized styling of some sort, I'll update the rules and redo this in a bit.
It will have scripts that can auto-format for those not using VS (or prefer to use a script) and will introduce a circleci check to ensure that the formatting is ok on future PRs
This will be a good first step in moving towards a slightly more organized development process including tests for adding to the changelog, adding tooltips, and (maybe, if this has enough traction) eventually running tests on FF1Lib itself