KSPModdingLibs / KSPBuildTools

MIT License
2 stars 4 forks source link

Improved Build Task CKANInstall to only execute CKAN if change was detected, allow helper mods with no reference to be auto installed. #22

Closed HB-Stratos closed 1 week ago

HB-Stratos commented 1 month ago

To make a testing environment easily reproduceable, I improved the CKANInstall target to the point where I believe it can be added as a BeforeBuildScript to be run. It can auto-install and remove mods from a game install depending on how the csproj is configured, allowing for helper mods and referenced mods. When nothing needs to be done, it only takes 19 ms to run, which I feel is acceptable. This isn't totally fleshed out yet, but as I was told @drewcassidy was working on this too, I figured I'd rather open my PR sooner than later. I hope it helps, even if this was my first time writing msbuild. Edits and improvements are more than welcome!

Example Usage

ModName.csproj:

[...]
  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
  <Import Project="$(SolutionDir)KSPBuildTools\KSPCommon.targets" />
    <!-- References to be included in the build -->
    <ItemGroup>
        <Reference Include="0Harmony">
            <HintPath>$(KSPRoot)/GameData/000_Harmony/0Harmony.dll</HintPath>
            <CKANIdentifier>Harmony2</CKANIdentifier>
        </Reference>
    </ItemGroup>

    <!-- Separate group for CKAN mods to install but not reference -->
    <ItemGroup>
        <CKANMod Include="KerbalEngineerRedux" />
        <CKANMod Include="KSPCommunityFixes" />
        <CKANMod Include="QuickStart" />
        <CKANMod Include="ModuleManager" />
    </ItemGroup>
[...]

Known Issues

drewcassidy commented 1 month ago

Ah, I was actually going to make it run after the Restore target. I don't think running ckan in the build script is a good idea

HB-Stratos commented 1 month ago

I would argue it is a good idea to have this in general. The restore target may be an idea, I am not entirely sure when that runs. But if it does run whenever the csproj is reloaded, that would be the perfect spot for it. Like that it would perfectly fulfill the goal I had of a fully parametric and reproducible dev environment, so anyone who clones a mod immediately gets the entire required toolset to work on it. No matter when the script is run though, I'd argue it's a good idea to speed the process up by only running ckan if necessary.

drewcassidy commented 1 month ago

Restore is run when dotnet restore is run. This is also where Nuget packages get installed so I'd say it's the canonical place to install dependencies in msbuild projects. It's still reproducible it just requires one extra step. It might get run in rebuild, I'll have to look that up but msbuild documentation is hot garbage

The mod developer can always add the ckan step to run on prebuild too. I'm wary of the additional moving parts the cache file you've added introduce though. I'll look into it more later.

Doesn't CKAN already keep a local list of installed mods?

JonnyOThan commented 1 month ago

Yeah restore seems like the right place, you don’t want to incur this overhead on every build.

drewcassidy commented 1 month ago

Definitely some good ideas here I'll incorporate, thanks for the suggestions!

drewcassidy commented 1 week ago

Well, turns out imports don't happen for nuget packages during restore so ckan-in-restore is completely incompatible with nuget packaging. Luckily I implemented the "only when a change is detected" independently while messing around so doing it during build isnt a problem, since the ckan command only gets executed if it has to.

Im going to close this since its been completely re-implemented now, but you get the satisfaction of knowing you had the right idea

HB-Stratos commented 1 week ago

Does the new implementation maintain my added feature of being able to specify helper mods that do not get added as references, only to make a reproduceable dev env?

drewcassidy commented 1 week ago

Yes