Skip to content

Conversation

@HolterPhylo
Copy link
Collaborator

No description provided.

@HolterPhylo HolterPhylo changed the title Array size info [MRG] Array size info Oct 10, 2025
@HolterPhylo
Copy link
Collaborator Author

HolterPhylo commented Oct 10, 2025

I changed "array=" to "array_size=" in "MXmlProperty.cs", but the output MXML is still showing "array=".
I must be doing something wrong or I forget some instruction to give...
Can you point me to the right track?

@HolterPhylo HolterPhylo changed the title [MRG] Array size info Array size info Oct 10, 2025
@monkeyman192
Copy link
Owner

Few general comments:

  • You have added summaries, but in some cases there is the wrong number of /'s (some places had 2 added), and also some summaries are just missing the information.
  • I think it should be type info, not typed info. The argument can be --typed, but the actual names and descriptions should be type info

@HolterPhylo
Copy link
Collaborator Author

HolterPhylo commented Oct 12, 2025

  • I commented out those places with 5 /, I did not want to remove them right away. I do not know enough about function/method definition to know if they are needed or how to write them correctly.

  • The summaries missing information is because I do not know what the information should be. Sure you can help.

  • Changed IncludeTypedInfo to IncludeTypeInfo.

@HolterPhylo
Copy link
Collaborator Author

I changed "array=" to "array_size=" in "MXmlProperty.cs", but the output MXML is still showing "array=".
I must be doing something wrong or I forget some instruction to give...
Can you point me to the right track?

I still need help here ^

@monkeyman192
Copy link
Owner

I changed "array=" to "array_size=" in "MXmlProperty.cs", but the output MXML is still showing "array=".
I must be doing something wrong or I forget some instruction to give...
Can you point me to the right track?

I still need help here ^

See suggested changes for fix...

image

@HolterPhylo
Copy link
Collaborator Author

HolterPhylo commented Oct 13, 2025 via email

@monkeyman192
Copy link
Owner

monkeyman192 commented Oct 14, 2025

I know we want array_size=‘4’ but I can’t get it to output that. See my comment and the file MXmlProperty.cs

sorry, github is weird and I didn't realise I needed to press a different button to submit the suggestions.
You should be able to just press a button to accept my suggested code changes and it will apply them and it should work.

@HolterPhylo HolterPhylo changed the title Array size info [MRG] Array size info Oct 15, 2025
@HolterPhylo
Copy link
Collaborator Author

Can you help me understand why "Array" did not work but "ArraySize" does?

@HolterPhylo
Copy link
Collaborator Author

Do you think any other corrections are required?

Comment on lines 89 to 94
/// <summary>
/// Writes the NMSTemplate object to an .mxml file.
/// </summary>
/// <param name="outputpath">The location to write the .mxml file.</param>
/// <param name="hideVersionInfo">version info is written to the MXML file.</param>
public static string WriteTemplate(NMSTemplate template) => WriteTemplate(template, false);
Copy link
Owner

Choose a reason for hiding this comment

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

Oh actually, I'm dumb. We should keep this chunk. I clearly didn't look close enough at it before saying we should remove it sorry...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When I add it back, VS does not see any reference to it.
Why do we need it?

@monkeyman192
Copy link
Owner

Ok, pending me doing one final round of testing, I think this looks good to go (other than that one comment I just left, sorry about that...)

@monkeyman192
Copy link
Owner

Ok! Looks good to me! Thanks for adding this 😄

@monkeyman192 monkeyman192 merged commit d94a754 into monkeyman192:development Oct 28, 2025
6 checks passed
@HolterPhylo HolterPhylo deleted the ArraySizeInfo branch November 13, 2025 22:25
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.

2 participants