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

[Feature]: Add support for using map IDs in macro functions #3852

Closed
kwvanderlinde opened this issue Mar 6, 2023 · 5 comments
Closed

[Feature]: Add support for using map IDs in macro functions #3852

kwvanderlinde opened this issue Mar 6, 2023 · 5 comments
Assignees
Labels
feature Adding functionality that adds value

Comments

@kwvanderlinde
Copy link
Collaborator

kwvanderlinde commented Mar 6, 2023

Feature Request

Since map names can be changed and aren't unique, they can't be used by libraries as stable identifiers for maps. Map IDs are better to refer to maps since they are unqiue and can't be changed by the user. But there is no support in the macro function for using map IDs, so even if a library author wants to use map IDs they first have to convert to non-unique map names.

The Solution you'd like

Whenever a map name or display name can be used as an input to a macro function, a map ID would also be accepted.

A few new macro functions would also be created:

  • getCurrentMapID(): get the ID of the current map.
  • getMapIDs(mapName, delim): get all the IDs of maps that have the name mapName, as a string list or JSON array.
  • getAllMapIDs(delim): get all the IDs of maps in the campaign as a string list or JSON array.

Finally, getInfo("campaign") would have a new key "zoneIDs whose value is an array of map IDs in the campaign. With @FullBleed's suggestion of the getAllMapIDs() function, this is no longer need to get that list.

Alternatives that you've considered.

Have the library add a special token to each map to contain metadata such as an ID for the map. The downside is that the token can get in the user's way and can be inadvertently deleted or modified by the user, thus losing the map ID. The library author also has to add extra logic to make sure these tokens exist, and to generate the IDs. And if more than one library needs map IDs, they either need to coordinate or produce their own tokens and IDs.

Additional Context

No response

@kwvanderlinde kwvanderlinde added the feature Adding functionality that adds value label Mar 6, 2023
@FullBleed
Copy link

FullBleed commented Mar 6, 2023

This will be a nice improvement.

While the getInfo method of rounding up all the ids is good, I think it would be nice to have a simpler getAllMapIDs option. Otherwise I know I'm just creating a UDF immediately to do that like I have for other getInfo data-points with "missing" functions. ;)

Oh, and shouldn't "ID" be capitalized in the functions? (i.e. "Photo ID" always capitalizes both letters, etc.)

@kwvanderlinde
Copy link
Collaborator Author

Good Idea, I'll add that function.

For the capitalization, I'll have you know my approach is the One True Way to camel humps abbreviations 😄 It doesn't matter for functionality since MTScript functions are (supposed to be) case insensitive, but your way is probably easier on most people's eyes when reading docs and such. I'll change that too.

@Azhrei
Copy link
Member

Azhrei commented Mar 6, 2023

I'm not a fan of increasing the MTscript footprint with YAFN (Yet Another Function Name) so I'm in favor of putting the info in the return value of getInfo(). If you want, it can be a different key, such as getInfo("MapIds") (or ZoneIds, or whatever). I'm not sure how well json.path can handle it, but maybe it'd be useful to have a JSON object that maps ZoneIds to MapNames and then another one that has MapName as the property and ZoneIds as a JSON array (with ZoneIds in the same order as the names appear in the Select Map list).

No idea how @cwisniew feels about it and it's really his call...

@kwvanderlinde
Copy link
Collaborator Author

kwvanderlinde commented Jun 23, 2023

There's a bug here that I need to fix. If a map name has exactly 32 characters but is not a valid GUID, a NumberFormatException goes unhandled, breaking some existing calls to these functions.

For any future testers, opening this 1.13.1 campaign will result in this error incorrectly being generated:

Event continuing after error running onCampaignLoad@Lib:Test: java.lang.NumberFormatException: not a hexadecimal digit: "T" = 84 error executing expression getOwned(getPlayerName(), "json", getCurrentMapName()).

@kwvanderlinde
Copy link
Collaborator Author

Closing this off as the feature has been released for a while now. Bug reports can be filed if needed.

@github-project-automation github-project-automation bot moved this from Needs Testing to Done in MapTool 1.14.0 Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding functionality that adds value
Projects
Status: Done
Development

No branches or pull requests

3 participants