Maptool: Token ids reset on server start

[Merudo]

Describe the bug
The token ids are changed after server start. This can lead to multiple inconveniences.

To Reproduce
Steps to reproduce the behavior:

1. Create a new map with Dragon token.
2. Type

[findToken("Dragon")]

1. Start server.
2. Type

[findToken("Dragon")]

1. Notice the two ids don't match up.

Expected behavior
The token ids don't change.

MapTool Info

• Version: 1.5.2, 1.5.3, 1.5.4 RC1
• Install: New

Desktop (please complete the following information):

• OS: Windows
• Version 10

Additional context
Token ID changes between saves
Token IDs Being Reset on Campaign Load
[FREQ] Static Token ID's

[Merudo]

PR #620 fixes this issue.

[Phergus]

The campaign-dir branch is supposed to also have a fix for token IDs changing. You might want to take a look and see what was done there. Can't say that I ever tested if that fix worked or not.

[Merudo]

Do you know what happened to that branch? Why wasn't a PR initiated at the end of March?

[JamzTheMan]

It was deemed to be put on hold (campaign directories) for reasons to be done later.

[Merudo]

Thanks for the reference to the campaign-dir branch.

I checked the campaign-dir branch, it does fix the issue. However, there are a lot of other changes that I can't really tell if they are related or not. Documentation is also poor with no issue # ever referenced.

There is also a fix for copy/paste tokens retaining their ids, however in the process it introduced a bugs when trying to paste a cut token more than once.

So I'm sure how much of it is usable, to be honest. But maybe @source-knights can help us understand what he has done?

[JamzTheMan]

Yea, I wouldn't necessarily say you need to copy that code. I just recall that it seemed like there were multiple places where the Token GUID would get changed is all, so as long as your think you can cover those, I'm good. Removing the set id method in Token class is probably a good start.

Some of those changes really spawned because he really needed unique id's for everything he saved to a file like tokens, macros, etc. Macro's have a macro uuid that are getting copied when token is copied as well (and needs a fix, those should be regenerated on the new token)

[Azhrei]

Macro's have a macro uuid that are getting copied when token is copied as well (and needs a fix, those should be regenerated on the new token)

Ideally, the cloned macro should refer to the original with a reference count so that any changes to a shared macro (refcount != 1) should cause it to be copied and the copy modified. Standard _copy-on-write_ functionality. That will benefit memory consumption and defer the cpu/memory load of copying the macros until they actually need to be copied. Probably gets tricky when it comes to saving/loading macros from an external file, as the reference count metadata needs to be taken into account. (We already do that for assets, but still...)

[rkathey]

Seems like a lot of work on macro ids for not much gain and memory is cheap.

'Someday' we're supposed to have most of the heavy-lifting macros stored off-token but referenced from tokens. I would rather see the time spent on the endeavor.

[Merudo]

Yea, I wouldn't necessarily say you need to copy that code. I just recall that it seemed like there were multiple places where the Token GUID would get changed is all, so as long as your think you can cover those, I'm good. Removing the set id method in Token class is probably a good start.

As far as I know it is only on server start and on cut & paste. The later is a little bit difficult so I think it should have its own issue and fix.

'Someday' we're supposed to have most of the heavy-lifting macros stored off-token but referenced from tokens

What would be the benefits of that over library tokens?

[Merudo]

Ideally, the cloned macro should refer to the original with a reference count so that any changes to a shared macro (refcount != 1) should cause it to be copied and the copy modified. Standard _copy-on-write_ functionality. That will benefit memory consumption and defer the cpu/memory load of copying the macros until they actually need to be copied. Probably gets tricky when it comes to saving/loading macros from an external file, as the reference count metadata needs to be taken into account. (We already do that for assets, but still...)

I think the macro uuid getting changed is another issue.

EDIT: Thanks for documenting that

[Azhrei]

Sorry, I didn't mean to imply the macro thing should be here; just documenting the preferred fix for anyone who comes along later and thinks, "Hm, I'll tackle that one..."

[rkathey]

What would be the benefits of that over library tokens?

The benefit is getting rid of Library Tokens. :)

The idea is for a code repository for macros (or user functions) that exists in a common area. This would make a lot of things easier for coding and maintenance. Thus you could have things like packages with tree views, etc. to view and update code more easily rather than having to go to a token on a map to code.

[source-knights]

The campaign branch is not for that fix, the token id fix was just needed
for the overall purpose of that branch: to store a campaign as an XML
files, so that each map, token, etc has its own file and can be handled in
git.

When people were complaining that the files change every time they save
without any change (so git would always think it was updated), I found that
token ID bug and implemented the workaround. The cut&paste behaviour is
more or less a feature, I wanted to fix it but based on discussions that
this will be rewritten with upcoming MapTool releases anyway and some guys
wanted to test the specific behaviour cause they liked it I left it as it
is.

If you want to remove the setId method have fun with it ;) I did not invest
any more time cause was told that all this will change with the next
MapTool anyway. So I stopped developing and just use my own fork :)

On Mon, 26 Aug 2019 at 23:56, Keith Athey [e-mail] wrote:

What would be the benefits of that over library tokens?

The benefit is getting rid of Library Tokens. :)

The idea is for a code repository for macros (or user functions) that
exists in a common area. This would make a lot of things easier for coding
and maintenance. Thus you could have things like packages with tree views,
etc. to view and update code more easily rather than having to go to a
token on a map to code.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/RPTools/maptool/issues/619?email_source=notifications&email_token=ADAGVM2XHBHIVNANJGPNFZ3QGRNSTA5CNFSM4IPPZEPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5F5T5Y#issuecomment-525064695,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADAGVM4HAQUPE63VVELVBCDQGRNSTANCNFSM4IPPZEPA
.

[Merudo]

Thank you very much for the explanation, @source-knights !

I was wondering why you had the macro uuids stay constant, but it makes a lot of sense if you goal was to avoid frivolous git updates with the new campaign structure you implemented.

I for one am a bit sadden the new campaign structure wasn't incorporated into develop. It would definitely helps a lot with the maintaining of frameworks.

[Phergus]

Flagging this as macro changes just so framework coders are aware of the change.

[Phergus]

Tested. Tokens now keep IDs across server starts. Closing.