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

Nicer organization #166

Closed
wants to merge 5 commits into from
Closed

Conversation

BreakinBenny
Copy link

I chopped off some size from these files, I think this'll be good for the actual assetsmv.pk3

@aufau
Copy link
Member

aufau commented Jun 11, 2023

So what's the size of assetsmv.pk3 before and after?

@BreakinBenny
Copy link
Author

The original assetsmv.pk3 is about 1.45 MB (1528124 bytes), and my PR should reduce that precise value to 1527480 bytes. I thought it was rather messy to have so many spaces in place of tabbing for the .menu files, and I used experience from modifying Return to Castle Wolfenstein's menu files.

Testing shows that nothing changes in-game.

@aufau
Copy link
Member

aufau commented Jun 11, 2023

Should or does? This number looks unlikely to me. Deflate compression in pk3 is very good at compressing indentation.

These files are the way they are for specific reason:

  • shader and menu files are formated with a special linter script and this is how they are meant to be indented unless someone changes the script.
  • .sp files use 3 space indentation. It's the same in original assets and there is no need to change it.
  • menu patch files are generated by special tool. messy indentation/spacing comes from original .menu files and leaving it as original makes it easier to track changes.

@BreakinBenny
Copy link
Author

My modification to the .sp shows that the 3-space indentation isn't necessary at all and that even 1-space indentation also works fine; the way Raven Software set them up means you can't utilize tabs, unlike the .str files Jedi Academy swaps them for, as I discovered while optimizing the text files for the total conversion of Jedi Outcast's campaign.

Then is there some documentation for manually making .menu_patch files?

@aufau
Copy link
Member

aufau commented Jun 11, 2023

Sure, indentation is not necessary for the parser. But source files have two recipients - computer program and a person reading/editing them in the future (often yourself). Indentation and other formatting is intended for the later and it's important too.

I don't understand the part about tabs.

There isn't documentation on menu patch files, sorry. It's a tiny project and we don't document everything. It's an internal feature and not meant for mods. We create them with a special tool that generates a diff between original and modified file.

Why do you need to make menu patch files yourself? Maybe I can help. But generally we don't recommend mods to use them, but rather simply overwrite original .menu files in a mod pk3. We would have done the same in jk2mv but wanted to be as much gpl as possible and original menu files are not GPL.

@BreakinBenny
Copy link
Author

BreakinBenny commented Jun 11, 2023

Sure, indentation is not necessary for the parser. But source files have two recipients - computer program and a person reading/editing them in the future (often yourself). Indentation and other formatting is intended for the later and it's important too.

I don't understand the part about tabs.

When you press the Tab key to shift it a good bit rather than utilizing multiple spaces. In a string file (used by Jedi Academy) you can have a string wrapped in quotation marks tabbed. In a "striped", it doesn't get recognized at all and thus the choice is to either space it out or not.

There isn't documentation on menu patch files, sorry. It's a tiny project and we don't document everything. It's an internal feature and not meant for mods. We create them with a special tool that generates a diff between original and modified file.

Why do you need to make menu patch files yourself? Maybe I can help. But generally we don't recommend mods to use them, but rather simply overwrite original .menu files in a mod pk3. We would have done the same in jk2mv but wanted to be as much gpl as possible and original menu files are not GPL.

In all honesty, handmade menu patches like this is a rather ridiculous thought. I did suppose that internal feature would end up being handy for other source ports which add or replace pieces of the original .menu's to enhance them in one way or another (I'm looking at ET: Legacy and RtcwPro, the latter especially if and when it gets a Quake3e, Quake 3 Kenny Edition, or even JK2MV-esque treatment!), whereas in this case it's merely for cleaning-up after the machine does its thing. I do however fully understand JK2MV may not supply even slightly modified Raven Software's stuff, hence why you people went this way to maintain legal status,

@aufau
Copy link
Member

aufau commented Jun 12, 2023

Ok so so everything explained, I'm gonna close the pr. If you have any questions in the future before making a pr, feel welcome to ask on discord or somewhere here github.

If you just want to make pr without asking - it's fine too but I can't promise it will be merged, like this one. The project has specific goals and rules (compatibility) and we've had people surprised that we didn't want to merge some features in the past. We always welcome contributions as long as they are accepted by the maintainers (mostly daggo and me nowadays), pass the review and provide a useful feature/bugfix without breaking compatibility.

@aufau aufau closed this Jun 12, 2023
@BreakinBenny
Copy link
Author

BreakinBenny commented Jun 12, 2023

I have done something similar for ET Legacy, mind you. They turned it down as well... including my fix for the Axis command post's UV mapping to de-mirror it because I was supplying the MD3 file (which wouldn't be covered by the license). It's fine though, they might sooner accept .objdata_patch, .script_patch, even .cfg_patch and .voice_patch systems.

I fail to see how this would break any compatibility in the end, unfortunately. You're welcome to show me here, or on Discord as well. (You can find me as BreakinBenny#4659)

@aufau
Copy link
Member

aufau commented Jun 12, 2023

No, none of your commits here break any compatibility.

It's basically the timeless "spaces vs tabs" dilemma. I tried but one can't just win or convince someone in such argument :)

In the end there is no bug to be fixed or feature to be added, it's pure source code maintenance and as a maintainer I've decided that what we've got so far (indentation in these files) is okay and there is no need to change it.

jk2mv discord is there: https://discord.gg/GWxxhYRg feel free to join. my username is fau#5006

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.

None yet

2 participants