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

Changed behaviour of adding token property. Added JSON export #4972

Merged
merged 6 commits into from
Oct 10, 2024

Conversation

bubblobill
Copy link
Collaborator

@bubblobill bubblobill commented Oct 2, 2024

Identify the Bug or Feature request

resolves #4768

Description of the Change

When there is a table row selected and adding a new property, the new property is inserted at the selected row
Exporting campaign properties now allows exporting as JSON
Added message on JSON export advising they cannot be imported.

Possible Drawbacks

none

Documentation Notes

Changed addTokenProperty behaviour to insert at selected row (where appropriate) instead of appending to the list.
Exporting campaign properties now allows exporting as JSON
Added message on JSON export advising they cannot be imported.

Release Notes

Adding a token property in campaign properties now adds the property at the selected row.
Exporting campaign properties now allows exporting as JSON.


This change is Reviewable

When there is a table row selected, new property is inserted at that row instead of being appended.
@bubblobill bubblobill changed the title Changed behaviour of adding token property. Changed behaviour of adding token property. Added JSON export Oct 2, 2024
@bubblobill
Copy link
Collaborator Author

#4649

@cwisniew
Copy link
Member

cwisniew commented Oct 3, 2024

@bubblobill maybe I am a bit daft, but whats the reasoning behind being able to export as JSON file if it can not be imported at a later time?

@fishface60
Copy link
Contributor

@bubblobill maybe I am a bit daft, but whats the reasoning behind being able to export as JSON file if it can not be imported at a later time?

I could use it in generating documentation for my add-on by exporting to a file and having my documentation include it and parse it to generate a table.

I'll probably be parsing the content.xml of the unzipped .mtprops file instead though since I version control the properties of my add-on that way.

@bubblobill
Copy link
Collaborator Author

@bubblobill maybe I am a bit daft, but whats the reasoning behind being able to export as JSON file if it can not be imported at a later time?

JSON is considerably more human readable than xml. I've seen numerous requests where people want to see what's inside. For folk that want to get to grips with what's inside the box it's a friendlier place than exporting/unzipping/parsing xml/ then writing your own and reversing the process.

@cwisniew
Copy link
Member

cwisniew commented Oct 9, 2024

@bubblobill maybe I am a bit daft, but whats the reasoning behind being able to export as JSON file if it can not be imported at a later time?

JSON is considerably more human readable than xml. I've seen numerous requests where people want to see what's inside. For folk that want to get to grips with what's inside the box it's a friendlier place than exporting/unzipping/parsing xml/ then writing your own and reversing the process.

yeah but all of these requests are so they can edit it :)

@bubblobill
Copy link
Collaborator Author

bubblobill commented Oct 10, 2024

It's a very non-obvious feature. You have to export the campaign and choose ".json" or ".txt" in the file filter dropdown to get it.
Choices:

  1. Leave it.
  2. Leave it and don't tell anybody it's there (undocumented feature)
  3. Leave it but remove the file filters so you have to explicitly name a ".json" target file (undocumented feature).
  4. Leave it and put a JSON import feature on the TODO list (tell me I'm not hilarious)
  5. Leave it, put a JSON import feature on the TODO list, and declare a new ".universal_campaign_one_size_fits_all_VTT_format" to the world so they have to adopt our format.
  6. Remove it

@cwisniew cwisniew added this pull request to the merge queue Oct 10, 2024
Merged via the queue into RPTools:develop with commit be5aa69 Oct 10, 2024
4 checks passed
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.

[Feature]: Classic Token Properties menu within Campaign Properties OR advanced "Add" feature.
3 participants