MapTool 1.3 Bugfix Build 79 (1.3-RC2)

New build announcements plus site news and changes.

Moderators: dorpond, trevor, Azhrei, Craig

User avatar
JonathanTheBlack
Dragon
Posts: 544
Joined: Mon Jun 28, 2010 12:12 pm
Location: I'm the worm...

Re: MapTool 1.3 Bugfix Build 79 (1.3-RC2)

Post by JonathanTheBlack »

PinkRose wrote:The only thing I see missing that I would think is trivial (but what do I know, I don't code) is a keyboard shortcut for "Lock Player Movement".
If that could get thrown in before a Final final build, that would be real swell.
I second this!

User avatar
Azhrei
Site Admin
Posts: 12086
Joined: Mon Jun 12, 2006 1:20 pm
Location: Tampa, FL

Re: MapTool 1.3 Bugfix Build 79 (1.3-RC2)

Post by Azhrei »

rwg wrote:So to expound on my original email...
setOwner("[]") This is an empty JSON array
setOwner("['']") This is a JSON array with one entry, an empty string
The code apparently doesn't check for empty strings as player names and it should. Such player names should produce an error and will in the next build. Empty will be defined similar to, "any string which, when leading and trailing spaces and tabs are removed, results in a string with length zero."

I'm reluctant to use a String parameter with some special code, such as "ALL", to mean that the All Players flag should be turned on. What's to stop a player from logging in as ALL?

I'm thinking that a true/false flag should be passed that sets the All Players setting. Perhaps something like:

setOwner(jsonList, id)
setOwner(jsonList, allPlayers, id)
setOwner(jsonList, allPlayers)

If the second parameter is zero or one, it's the All Players setting. Otherwise it's a token id. If the All Players setting is not specified, it defaults to false (zero). Setting it to true (one) would cause the first parameter to be ignored and all owners to be turned off.

So existing code would turn off the All Players setting and only new code that knows about the second parameter would be able to turn that setting on.

Thinking about it now, it seems we have a few requirements. First, the assumption that All Players is never valid when player names are set as owners. Second, the assumption that for only the GM to have access all other information should be removed (All Players and player names).

1. Turn off All Players and remove all player names (essentially GM-only)
2. Turn off All Players and set individual players
3. Turn on All Players and remove all player names

We could use the following syntax for each one:

setOwner("[]") - obviously removes all players and somewhat-obviously removes All Players

setOwner("[...]") - obviously sets certain players as owners and somewhat-obviously removes All Players

setOwner("") - obviously removes all players and not-so-obviously turns on All Players :?

Any non-empty string provided as the first parameter MUST be a JSON array; anything else produces an error. This means that existing code that tries to set a single owner would be flagged as an error and would need to be changed to create a JSON array with a single element instead. This doesn't thrill me (backward compatibility and all that) and I'd be happy with a non-empty string being treated as a JSON array with one element.

Given that each of the above would be documented, are there any problems?

rwg
Kobold
Posts: 16
Joined: Fri Mar 19, 2010 5:51 pm

Re: MapTool 1.3 Bugfix Build 79 (1.3-RC2)

Post by rwg »

Dead horse beating time: I agree that passing in “All” would be a poor key word, but a longer word such as “AllPlayers” seems acceptable, certainly more intuitive than using “” for the purpose.
But overall your suggestion seems functional.

Another option is to give the poor setOwners() function a rest by creating a new function:
setOwnerAllPlayers(bool)
setOwnerAllPlayers(1) -- clears names, sets AllPlayers flag.
setOwnerAllPlayers(0) -- clears AllPlayers flag

In this way set Owners can revert to original backwards compatible code (except for new code to error when detecting blank names in the JSON).
setOwners(“”) -- clears just the names (leaves AllPlayers flag alone)
setOwners(“John”) -- sets John and Dave, clears the AllPlayers flag
setOwners(“['John', 'Dave']”) -- sets John and Dave, clears the AllPlayers flag

But this introduction of a new function might be riskier for final build than 'fixing' the old code.

Craig
Great Wyrm
Posts: 2107
Joined: Sun Jun 22, 2008 7:53 pm
Location: Melbourne, Australia

Re: MapTool 1.3 Bugfix Build 79 (1.3-RC2)

Post by Craig »

With any changes you make you must ensure that a player can not change the token to be owned by no one (or just the GM). As this would allow them to create non player lib tokens

User avatar
Azhrei
Site Admin
Posts: 12086
Joined: Mon Jun 12, 2006 1:20 pm
Location: Tampa, FL

Re: MapTool 1.3 Bugfix Build 79 (1.3-RC2)

Post by Azhrei »

Craig wrote:With any changes you make you must ensure that a player can not change the token to be owned by no one (or just the GM). As this would allow them to create non player lib tokens
Oh, good idea. I hadn't considered that. I will need to verify the changes I made to setOwner previously and I'll add a comment to the Java code so that people with their hands in that area of the code in the future are warned.

Thanks!

Post Reply

Return to “Announcements”