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

Refactored mod creation to use a file tree of templates. #4134

Merged
merged 23 commits into from
Apr 10, 2024

Conversation

Mirsario
Copy link
Collaborator

@Mirsario Mirsario commented Mar 24, 2024

To assist two+ active PRs (#2472 and #4037), this one does the following:

  • Moves code related to creating & upgrading mod source folders out of UI and into a new separate Terraria.ModLoader.Core.SourceManagement class.
  • Replaces textfile-generating hardcode with a filetree of embedded resources that are enumerated and exported to populate mod src folders. The embedded resources use the Mustache logicless template format in their names and contents (this concept will be nice to use for mod descriptions too in userland in the future). Optional files, such as the BasicSword, are skipped by making their filenames evaluate to nothing.
  • Removes TargetFramework and PlatformTarget from default .csproj files, putting it into tModLoader.targets tMLMod.targets instead.
  • Makes Csproj upgrades attempt to function via XML edits, if they're not too broken.
  • When this hits Stable, update https://github.com/tModLoader/tModLoader/wiki/Basic-tModLoader-Modding-Guide#mod-skeleton-contents info and screenshots with new Content.Items namespace and folder.

Default mod template changes:

  • launchSettings.json, uses tabs now.
  • Csproj uses tabs now, no longer has an obsolete <?xml> header, is now practically empty besides for the targets import, with every other line just being nice "put this here" markups.
  • The item template now uses the ExampleSword.png, which is nicer to look at. A tooltip has also been added, to immediately showcase how to make multiline strings in HJSON.
  • The two .cs files now use file-scoped namespaces. The item template's code had its comments updated, and had magic numbers replaced with ID classes in a few lines.
  • description.txt:
-{{ModDisplayName}} is a pretty cool mod, it does...this. Modify this file with a description of your mod.
+Modify this file with a description of your mod.
  • As too many people don't bother even providing a mod icon, I felt like updating the default mod icon to be slightly more presentable, while communicating its placeholder nature:
    ->

Questionable notes:

  • I've had to exclude the new template folder from AssemblyResourcesContentSource, because of it rather annoyingly crashing the game whenever it encountered a .cs + .png pair.
  • The templates' EmbeddedResource LogicalNames are set to use forward slashes, due to localization files utilizing dots, which slashes get reasonlessly converted to by default. Ideally even the vanilla game would convert to using forward slashes, it's beneficial for its asset system.

@Destructor-Ben
Copy link
Contributor

In a future PR could we allow specifying our own template? atm I copy paste a template and hope I remembered to change all file names + contents of files

@JavidPack
Copy link
Collaborator

Should we move LangVersion to the tMLMod.targets too? I believe a modder should still be able to override it in their mod's csproj, but an issue we have right now is that modders are seeing suggestions in VS for language versions that the ingame complier does not support. I know there is a pr for doing it through dotnet itself, but even then we'd want to make sure that only the highest supported lang version is the default.

@Mirsario Mirsario force-pushed the refactor/mod_creation branch from 24bebf1 to d2bb31b Compare March 26, 2024 00:06
@Mirsario
Copy link
Collaborator Author

Should we move LangVersion to the tMLMod.targets too?

I've done that now, with it explicitly set to 10.0 for now. But only overrides with old versions in values will be auto-removed in the upgrade process. latest, preview, and >=11.0 will be kept, so I guess it's not really helping what you've described. Oh well, hopefully we merge 2472 soon.

@JavidPack
Copy link
Collaborator

Will this be able to easily remove <PackageReference Include="tModLoader.CodeAssist" Version="0.1.*" /> once #4099 is merged? I haven't parsed all the code yet, just curious.

@Mirsario
Copy link
Collaborator Author

Mirsario commented Mar 26, 2024

Yes, here's the code to insert even:

// Remove the analyzer package, since our targets file now handles all that.
RemoveNodes(itemGroups.Elements("PackageReference").Where(
	e => e.Attribute("Include")?.Value == "tModLoader.CodeAssist"
));

EDIT: This is now a part of the PR, with the PackageReference moved into tMLMod.targets.

Copy link
Member

@Chicken-Bones Chicken-Bones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this is an excellent change. I've provided some factoring feedback.

This PR should not change the templates. It makes it really hard to review. Please split template changes into an additional 1 or 2 PRs (2 if there are changes that have to be done both before and after this PR).

@Mirsario Mirsario force-pushed the refactor/mod_creation branch from 90dfa90 to 454247d Compare April 9, 2024 12:54
@Mirsario Mirsario requested a review from Chicken-Bones April 9, 2024 13:12
@JavidPack JavidPack merged commit dde87b4 into 1.4.4 Apr 10, 2024
@JavidPack JavidPack deleted the refactor/mod_creation branch April 10, 2024 23:11
Solxanich pushed a commit that referenced this pull request Apr 15, 2024
* Refactored mod creation to use a file tree of templates.

* Upgrade .csproj files via XML edits when possible.

* Include TargetFramework & PlatformTarget in 'tMLMod.targets'.

* Add LangVersion to .targets, remove old ones from csproj files.

* Major code & design improvements.

* Slightly less convoluted XElement matching.

* Even less convoluted XML Linq.

* Reduce template .csproj.

* One less line for TML .csproj.

* Made TemplateParameters a ref type.

* Fixed and reduced launchSettings.json template.

* Handled some SourceManagement feedback.

* Finish addressing SourceManagement flow feedback.

* Auto-remove 'tModLoader.CodeAssist' from csproj files, since it's now in targets.

* Updated for .NET 8.

* Use .bak extension for csproj backup

* No file scoped namespaces yet. Link to stable branch and wiki.

* Fix template files not being embedded

* Fixed regressions in CollectCsprojModifications.

* Don't remove conditional item & property groups.

* Fix ResetCsprojFile

---------

Co-authored-by: Chicken-Bones <mehvids@gmail.com>
Co-authored-by: JavidPack <javidpack@gmail.com>
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