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

[Bug]: getMapDisplayName() returns error when map does not have a display name #3857

Closed
kwvanderlinde opened this issue Mar 10, 2023 · 8 comments · Fixed by #3861
Closed
Assignees
Labels

Comments

@kwvanderlinde
Copy link
Collaborator

Describe the Bug

If a map has no display name, the getMapDisplayName() macro function will result in an obscure error:

  Error in body of roll.
    Statement options (if any): r
    Statement Body : getMapDisplayName()
Error trace : chat

To Reproduce

  1. Start a new campaign
  2. Type this into chat: [r: getMapDisplayName()]
  3. See the above error

Expected Behaviour

getMapDisplayName() should return an empty string when no display name is set.

Screenshots

No response

MapTool Info

develop

Desktop

Linux Mint 21.1

Additional Context

This does not affect 1.12.2 where getMapDisplayName() would return the map name if no display name was set. It looks like the behaviour was introduced in e028925.

@kwvanderlinde
Copy link
Collaborator Author

A perhaps better alternative would be to return the map name if no display name is set.

@FullBleed
Copy link

A perhaps better alternative would be to return the map name if no display name is set.

If it doesn't do this, I suspect someone will ask for it... ;) But, I think if a map has no assigned display name, then we should have a way of knowing that... so an empty string would be the best option.

@kwvanderlinde
Copy link
Collaborator Author

Just realized this is also related to another quirk I've noticed (but keep forgetting about) where the loading message for a map with no display name shows something like "Loading Map 'null' - ..."

@kwvanderlinde
Copy link
Collaborator Author

A perhaps better alternative would be to return the map name if no display name is set.

If it doesn't do this, I suspect someone will ask for it... ;) But, I think if a map has no assigned display name, then we should have a way of knowing that... so an empty string would be the best option.

That's what I thought at first too, but I realize now I was too wrapped up in the internals. From a user perspective, a map always has a display name. It's just that the display name might be the same as the regular name. So it would be surprising if the macro function returned an empty string just because a separate display name isn't set internally, especially given that the user will see a display name set, for example, when they open the Edit Map dialog.

@FullBleed
Copy link

Then if we can't pull an empty string then it seems to be that the solution is that the Display Name should never be able to be empty. It's either the Map Name or explicitly set to something else.

@thelsing
Copy link
Collaborator

Then if we can't pull an empty string then it seems to be that the solution is that the Display Name should never be able to be empty. It's either the Map Name or explicitly set to something else.

This was changed in dev because of a FREQ. I don't really care what the macro returns, but I don't want to have to set two names on a map.

@FullBleed
Copy link

This was changed in dev because of a FREQ. I don't really care what the macro returns, but I don't want to have to set two names on a map.

That's fair.

People shouldn't have to fill in the field. It has its uses, but isn't universal.

In the PR that set it to be Blank when not being used (I think that was done to stop some other errors from being thrown), @Phergus mentioned that:

"The Display Name could be updated to match when the GM Name is changed to get the visual confirmation of the change."

I think I'd prefer that option. i.e. Whatever you type into the Map Name auto-fills the Display Name field IF it is empty. Otherwise, leave the Display Name alone.

That way the Display Name could never be empty. It's either the Map Name or explicitly set to something else.

Then we don't have this error or the other load-in null error @kwvanderlinde noted above.

@kwvanderlinde kwvanderlinde self-assigned this Mar 11, 2023
@kwvanderlinde
Copy link
Collaborator Author

This was changed in dev because of a FREQ. I don't really care what the macro returns, but I don't want to have to set two names on a map.

I definitely agree with that. I'll make sure I keep the display name blank when not needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants