RobustToolbox
RobustToolbox copied to clipboard
Xml documentation comments imported into View Variables
Overview
Adds code that can read the xml files generated by C# documentation comments and display those comments (currently the <summary> block) below types and variables in View Variables windows.
~~This PR doesn't (yet) actually toggle the xml doc file generation. You should add <GenerateDocumentationFile>True</GenerateDocumentationFile> to the csproj of the project you want to generate docs for then copy the resulting xml file into Resources/ViewVariables. You can use this to test this PR with the xml docs from content as well.~~
I will do a content PR when this gets merged but to test you need to add:
<ViewVariables>true</ViewVariables>
<VVRobustProjectDir>RobustToolbox</VVRobustProjectDir>
to the Content csproj files to get them generating the doc files correctly.
~~The xml docs are a little big, and they contain a bunch of info we're never going to use like function documentation. It wouldn't be to hard to have a script that discards unneeded entries to optimize the size of the resulting files.~~
All the individual doc strings are loaded from the client resources folder, we don't send the doc strings over the network.
Since this info comes from the code (just like the type and variable names) we can't really expect to localize this text. That may be a deal-breaker.
TODO
- [x] fix xml tags inside the
<summary>block breaking the doc string.- Most of the tags are now good enough, I would like to try implementing
<inheritdoc/>but it might be in a follow-up PR.
- Most of the tags are now good enough, I would like to try implementing
- [x] actually add
<GenerateDocumentationFile>to the csproj files.- I spent some time in the MSBuild mines and I found a way to get the xml doc to write to the
Resourcesfolder cleanly. Right now it's only enabled for the Robust DLLS. If you want to test it locally you can add<ViewVariables>true</ViewVariables>to the Content DLLS. ~~You may need to an an emptyResources\ViewVariables\folder for the build to work properly.~~
- I spent some time in the MSBuild mines and I found a way to get the xml doc to write to the
- [x] post-compile script to move the generated xml files into the Resources folder
- See above.
- [x] post-compile process to trim down the size of the xml files (discard all method definitions)
- XmlDocTool implemented and working 👍
- [x] Add support for
<viewvariables>block inside the xml comments that can add docs tailor-made for vv
~~[ ] cleaner code~~ ~~[ ] (?) CVar config?~~ ~~[ ] (?) Improve the UI slightly to better accommodate the new doc strings~~
insanely zased
xmldoc should probably be gitignored since it'll be regenerated anyway
xmldoc should probably be gitignored since it'll be regenerated anyway
Yes I was hoping someone would weigh in on this. ~~If we want to do this we need a way to keep Resources\ViewVariables from being deleted since the build script can't (yet?) create the folder if it does not exist.~~
~~Actually scratch that last, I can make msbuild create the folder apparently~~
There's now an command line program called XmlDocTool that strips function information out of the docs since we wont ever use that data.
The number of excluded entries is pretty dramatic:
XmlDocTool - Robust.Shared: Total Nodes: 4864, Copied Nodes: 2157, Delta: -2707
XmlDocTool - Robust.Client: Total Nodes: 1583, Copied Nodes: 918, Delta: -665
XmlDocTool - Robust.Server: Total Nodes: 318, Copied Nodes: 122, Delta: -196
XmlDocTool - Content.Shared: Total Nodes: 14220, Copied Nodes: 5285, Delta: -8935
XmlDocTool - Content.Client: Total Nodes: 1771, Copied Nodes: 639, Delta: -1132
XmlDocTool - Content.Server: Total Nodes: 12097, Copied Nodes: 3475, Delta: -8622
Ok there are some minor pain points remaining (gui is a bit scuffed when the comments are big and long), but I think I'm ready for this to exit draft.
I merged latest and did some tests, so far as I can tell this is still ready to go. I think this would really encourage people to write more/better DocStrings. Let me know if I should write a bit more technical documentation about how this works. Even if I'm personally not very active (which I want to change) I think this should be pretty easy to maintain.
Depending on events in my personal life I may or may not be able to support this, it would really suck to see it discarded so here are some more details in case I'm unavailable to do it myself.
Instructions for integrating this into Content:
- Add the
Robust.XmlDocToolproject to theSpaceStation14.slnsolution - Update the following
csproj:Content.Server.csprojContent.Shared.csprojContent.Client.csproj- Add the following lines to the
<PropertyGroup>section:
<ViewVariables>true</ViewVariables>
<VVRobustProjectDir>RobustToolbox</VVRobustProjectDir>
After that it should basically just work™
- Set up the content
.gitignoreto ignoreResources/ViewVariables/since it's program-generated MSBuild/Robust.Properties.targetsgives a good outline of how this all works. In the grand scheme of things it's relatively uncomplicated, essentially just copying data selectively from one place to another.
Downstream concerns:
It occurred to me that this whole feature might be undesirable for certain downstreams that use private content dlls. Obviously any client will have the Shared and Client dlls downloaded and they could be inspected with ILSpy or similar, but I imagine that there could be details left in the comments in both Client and Shared dlls that downstreams would prefer (and currently expect) to be kept private. Of greater concern however is that this will leak details of the Server dll to clients in the form of the scraped comments and the names of the types those comments apply too.
Here are a couple of possible mitigation strategies:
- Make the whole feature only work in debug mode:
- This is the easiest but also least desirable outcome.
- Anyone who knows a bit about MSBuild should be able to modify this so the whole process is only enabled in debug mode.
- In my head I've always imagined this not just as a feature to support development, but also supporting admins who are using vv in a live situation for whatever reason (admin shenanigans or live debugging). So I think one of the following solutions would be better.
- Tell downstreams to edit the csproj files to their taste:
- If/when this gets merged, an announcement that it has been added is definitely warranted.
- If the feature gets enabled in content by default, include instructions in the announcement on how downstreams can disable the behavior (downstreams should just remove the
<ViewVariables>true</ViewVariables>line from the csprojs they don't want to expose).
- Create a new
.targetsfile for downstream csproj settings.- This is an extension of solution 2. The small problem with solution 2 is that it means that downstreams will now need to merge any upstream edits to the csproj files any time they are updated.
- To allow downstreams to disable this feature and also make it very low friction during merges, the content csproj files should be updated to
<Import>a new.targetsfiles called i.e.Custom.Properties.targetswhich would in turn have the<ViewVariables>settings in it by default. downstreams could edit this file instead, which would be far less likely to get stomped by upstream, meaning that they could disable the feature's behavior while also making any future merges with the upstream csprojs seamless.
- Server-only Resources
- AFAIK this isn't currently possible
- Basically the generated doc files would only exist on the server and not be bundled with the client resources.
- Then the docstrings could be sent only to players with vv permissions.
- There might be a way to do this without blowing up a lot of the assumptions around Resources, but if there is I'm not currently in a position to investigate a solution.
There is a bunch of stuff going on in my life right now that I need to sort out before I can decide if I have the energy to fully come back to ss14. In the meantime though I should be able to answer any questions about this PR that people might have.
One day the best QOL dev/admining PR will be merged... One day...