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

Minify meta.json files #13035

Closed

Conversation

metalgearsloth
Copy link
Contributor

@metalgearsloth metalgearsloth commented Dec 15, 2022

These were always supposed to be minified we just got lazy. Also a handful of files were saved as utf-8 bom which I have updated to utf-8 (all of the cigar files + a handful of othe rones).

I also included the script I used to do it + checked it ingame.

These were always supposed to be minified we just got lazy.
@github-actions github-actions bot added the No C# For things that don't need code. label Dec 15, 2022
@github-actions github-actions bot added the Merge Conflict This PR currently has conflicts that need to be addressed. label Dec 15, 2022
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@mirrorcult
Copy link
Contributor

i really hate it if we're doing this, these are way harder to edit and rsiedit doesn't emit them minified anymore (+ people are unlikely to want to do if they're not using rsiedit) so it's just going to pile up again

what is the motivation, just reduced line count? i don't really think it matters

@metalgearsloth
Copy link
Contributor Author

If we do intend these to be hand editable now then it should just be yml like everything else.

@DrSmugleaf
Copy link
Member

DrSmugleaf commented Dec 15, 2022

rsiedit doesn't emit them minified anymore

Minified JSONs is a setting in RSIEdit, it could be enabled by default.
https://github.com/space-wizards/RSIEdit/blob/a4a1dd9c66607cee3909908e848157c643e0ae6e/Editor/Models/Preferences.cs#L24

@github-actions github-actions bot added the Status: Needs Review This PR requires new reviews before it can be merged. label Dec 15, 2022
@moonheart08
Copy link
Contributor

If we do intend these to be hand editable now then it should just be yml like everything else.

We've established multiple times they should be YML.
wyci

@PJB3005
Copy link
Member

PJB3005 commented Dec 15, 2022

We've established multiple times they should be YML.

Please no I don't want to have slow-ass YAML loading even more. Also YAML libraries in e.g. Python are all garbage compared to YamlDotNet.

If you wanna edit minified JSON files just install something like this into VSCode. One command to format the JSON (so it's readable) and another to pack it back down when you're done.

Also if we don't wanna minify the JSON at least change the editorconfig to match the current style we have (2 space indents).

Also merge #12997 first please I beg

@DrSmugleaf
Copy link
Member

DrSmugleaf commented Dec 15, 2022

We've established multiple times they should be YML

when

@metalgearsloth
Copy link
Contributor Author

what is the motivation, just reduced line count?

Texture folder size goes down 1.5MB

There's existing tooling around this like PJB said so I don't understand why pressing a keybind to unminify and another to minify is a big deal if you really need it which I was doing in #13027

We already have much bigger workflow difficulties than this that also don't give us free performance and can't be fixed with 1 button.

@AJCM-git
Copy link
Contributor

AJCM-git commented Jan 7, 2023

The maintainer council has deemed this as cringe as it would make things harder to edit and review for no real gain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge Conflict This PR currently has conflicts that need to be addressed. No C# For things that don't need code. Status: Needs Review This PR requires new reviews before it can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants