Xml documentation comments imported into View Variables by Mervill · Pull Request #5430 · space-wizards/RobustToolbox

Mervill · 2024-09-08T04:18:18Z

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>

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.~~

XML docs generated for server dlls get generated into /Resources/Server/ and that file is ignored during the client content build. Server docs get sent over the wire to users with VV access. Client and Shared xml docs get bundled into the client content package and are thus always visible.

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

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.
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 Resources folder 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 empty Resources\ViewVariables\ folder for the build to work properly.~~
post-compile script to move the generated xml files into the Resources folder
- See above.
post-compile process to trim down the size of the xml files (discard all method definitions)
- XmlDocTool implemented and working 👍
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~~

EmoGarbage404 · 2024-09-08T04:19:57Z

…o me about msbuild

Mervill · 2024-09-10T05:48:54Z

mirrorcult · 2024-09-10T22:30:39Z

insanely zased

mirrorcult · 2024-09-10T22:31:38Z

xmldoc should probably be gitignored since it'll be regenerated anyway

Mervill · 2024-09-10T23:51:40Z

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.~~

Mervill · 2024-09-10T23:54:38Z

~~Actually scratch that last, I can make msbuild create the folder apparently~~

…sbuild

…ated xml docs

…et the tempFilename

…rns a real error code

metalgearsloth

sure

…n the server and only be sent to users with vv access.

Mervill · 2026-10-20T23:35:15Z

2026 Update

I didn't manage to prepare in time for the Sep 7th anniversary of the original pr. c'est la vive.

The configuration instruction for this is now simply to add:

<ViewVariables>true</ViewVariables>

To a csproj to have it's xml generated and loaded into VV.

Client and Shared xml is copied to /Resources/ViewVariables/ and that xml can be inspected by clients directily (like client and shared dlls).
Server xml is copied to /Resources/Server/ViewVariables/ and /Resources/Server/ is ignored by the client packager. Server XML is then sent on-demand to clients that have VV permissions. This should resolve the issues around leaking the docs to unauthorized parties.
Copying is based on a dll ending with .Server but you can manually toggle the behavior with <ViewVariablesServerOnly>true</ViewVariablesServerOnly>

Inflates the warning count to 681. Due to some xml in the project being invalid. I have a follow up PR to address most of these. We ignore CS1591;CS1573 for now because we don't yet require xml docs. The warnings that show up now are broken formatting / bad references in xml.

This is required to stop everything from wobbling when XMLDoc is too long, and makes sure it's always displaying everything.

Bold looked back, DarkGray was good, just now using white more.

Fildrance

Okay so my comments are mostly minor things, there are several things up in the air but considering this is quite niche feature and it is okay to drop it as a matter of graceful degradation (i wish we had one for perf but i mean feature-richness wise, if it crashes of fails - not so much of a problem, game is still playable, unless it crashes the game).

I got a little bit tired to the end of the thing, and i felt really lacking in understanding what is multithread-threat engine-wise there, and what is okay to be - i know a lot of stuff is running on single thread, but there are async interactions, but i am just lacking knowledge on how generally engine works thread-wise (considering access to stuff like UI elements from different threads, etc). I feel like engine in general does not have PROPER tooling to do what you are doing, but building full idiot-proof infrastructure just for that feature seams insane, so i guess we are fine there?

Its nice if you could apply some stuff i proposed, and we could try think about solutions to problems i mentioned, or they could be not really a problem <_<

Fildrance · 2026-10-29T11:44:45Z

+            var missingKeys = new List<string>();
+            var missingIdx = new List<int>();


could it be better to use HashSet for those?

Fildrance · 2026-10-29T11:57:15Z

+
+            if (missingKeys.Count == 0 || !_netManager.IsConnected)
+            {
+                foreach (var i in missingIdx)


if missingKeys.Count == 0 , then missingIdx is empty, so that loop would effectively is doing nothing (probably separate early return is better?). And i am a bit confused how _netManager.IsConnected flag is working here... why would we need to do any of this if we are not connected?

Fildrance · 2026-10-29T12:05:03Z

+            {
+                var count = Math.Min(MsgViewVariablesReqDocStrings.MaxKeys, missingKeys.Count - start);
+                var chunk = missingKeys.GetRange(start, count);
+                var remote = await RequestDocStrings(kind, chunk);


I would propose naming it promise or task, or something along those lines.

Fildrance · 2026-10-29T12:09:01Z

+            {
+                var count = Math.Min(MsgViewVariablesReqDocStrings.MaxKeys, missingKeys.Count - start);
+                var chunk = missingKeys.GetRange(start, count);
+                var remote = await RequestDocStrings(kind, chunk);


Having no parallelism here is a bit odd. Should there be? From what i see that should not really be a problem, and doing parallelism here would be a dirty thing in itself, not trivial. Considering the only thing that currently would ask for batches is server-side components... should not be a problem i presume?

Fildrance · 2026-10-29T12:20:22Z

+
+            for (var i = 0; i < targets.Count; i++)
+            {
+                targets[i].Apply(docStrings[i]);


As this can contain basically anything - i would propose a bit of redundancy - wrapping whole thing in try-catch and logging any suspicious stuff. Otherwise async void method will fall silently and you will never know whats going on.

… generation.

This is needed to make things wrap correctly.

Aeshus · 2026-10-29T22:46:23Z

Here's the stylistic changes we discussed in the Discord.

As a future TODO that I didn't have time to complete (non-blocking):

Have the header also use a collapsible so that the docs there are hidden unless you click to expand them. Also fix the word clipping and the label stuff that it currently is doing.
Also make sure the header documentation text is using a RichTextLabel

This did require exposing the CollapsibleHeading's internal BoxContainer, but that's a fine change IMO and is useful for other things.

Also, this required the VV window to disable horizontal scrolling as otherwise text would not wrap.

… into vv-docstring

Mervill added 2 commits September 7, 2024 02:04

docstrings in view variables mvp

1343168

working with fields and properties

5e745f2

Mervill added 8 commits September 7, 2024 23:01

move this code since we'll be expanding it

457671d

Merge branch 'master' into vv-docstring

26a7115

boy I sure do love processing xml, yep.

a5a4fdc

not as much string cleanup as I expected

349c431

more robustness and cleanup

2aa841d

I met a man without a name in a meadow beyond time and he whispered t…

732305a

…o me about msbuild

comment

3637c6e

minor tabs-vs-spaces skirmish, spaces victorious

a19b647

Mervill added 14 commits September 10, 2024 19:41

contemplate the sound of wisdom, recover incremental knowlage about m…

0fda1a7

…sbuild

update gitignore

7d4cd92

slightly cleaner defensive programming

cd2e65f

Add XmlDocTool - a utility program for reducing the size of the gener…

e23bbbd

…ated xml docs

Move XmlDocTool into a Robust folder/namespace. use Path.Combine to g…

2a80d50

…et the tempFilename

add support for the viewvariables custom block in xml comments

887f63f

Merge branch 'master' into vv-docstring

7a62c0a

error format and return code in xmldoctool

64226ce

xmldoctool output path

63fecf8

run xmldoctool as a post build event

dc6b30b

Merge branch 'master' into vv-docstring

e87debc

better msbuild spellcasting

0c4376e

the ritual is complete, the line has folded and become a circle

727ecf9

update comments

967d33e

Mervill removed the request for review from PJB3005 August 25, 2026 21:21

Mervill added 4 commits October 15, 2026 15:37

Merge branch 'master' into vv-docstring

a9a719f

put all the vv stuff back following the merge

e0b8f03

Add GetDocStringForType(Type type)

f06dcd4

Turns out we don't need VVRobustProjectDir

8e21e9e

Mervill force-pushed the vv-docstring branch 2 times, most recently from c1401b3 to 8e21e9e Compare October 16, 2026 00:10

Mervill added 3 commits October 15, 2026 17:48

vv docstring will lazy load instead of direct load

7f5f768

xmltool overwrites directily instead of using a temp file, also retru…

8521bfa

…rns a real error code

do xmldoc tool in away that appeases certain important djinn (git cli)

0a76011

metalgearsloth approved these changes Oct 20, 2026

View reviewed changes

Mervill added 2 commits October 20, 2026 15:14

refactor to allow xml docs for server-only code to live exclusively o…

da51c88

…n the server and only be sent to users with vv access.

when the pipieline is sus

029de73

Fildrance self-assigned this Oct 28, 2026

Aeshus added 4 commits October 28, 2026 18:11

refactor: use RichTextLabel for word wrapping

e7c4f12

This is required to stop everything from wobbling when XMLDoc is too long, and makes sure it's always displaying everything.

feat: expose collapsible's BoxContainer

29df604

feat: make VV prettier

a059e3d

fix: back to labels

2801d4c

Bold looked back, DarkGray was good, just now using white more.

Fildrance suggested changes Oct 29, 2026

View reviewed changes

This comment was marked as duplicate.

Sign in to view

Mervill and others added 4 commits October 29, 2026 14:54

stealth fix for a common warning that appears because of enabling doc…

3ebc0a8

… generation.

better flow for the xmldoctool

ed75c57

fix: disable hscroll in VV windows

e3b47ad

This is needed to make things wrap correctly.

style: fix formatting

746714a

Mervill added 4 commits October 29, 2026 16:32

Merge branch 'vv-docstring' of https://github.com/Mervill/RobustToolbox…

8e669fa

… into vv-docstring

ViewVariablesDocStrings cleanup p1

c9bf109

beep

f9b390e

Harden the doc request againt some malicious cases.

da7ce79

		var missingKeys = new List<string>();
		var missingIdx = new List<int>();

Conversation

Mervill commented Sep 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

TODO

Uh oh!

EmoGarbage404 commented Sep 8, 2024

Uh oh!

Mervill commented Sep 10, 2024

Uh oh!

mirrorcult commented Sep 10, 2024

Uh oh!

mirrorcult commented Sep 10, 2024

Uh oh!

Mervill commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mervill commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metalgearsloth left a comment

Choose a reason for hiding this comment

Uh oh!

Mervill commented Oct 20, 2026

Uh oh!

Fildrance left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fildrance Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

Fildrance Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

Fildrance Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

Fildrance Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

Fildrance Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as duplicate.

Uh oh!

Aeshus commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Mervill commented Sep 8, 2024 •

edited

Loading

Mervill commented Sep 10, 2024 •

edited

Loading

Mervill commented Sep 10, 2024 •

edited

Loading

Aeshus commented Oct 29, 2026 •

edited

Loading