RPTools / maptool

Virtual Tabletop for playing roleplaying games with remote players or face to face.
http://rptools.net
GNU Affero General Public License v3.0
784 stars 259 forks source link

Enhancement: Macro functions to dynamically create light source and manage same #331

Open Phergus opened 5 years ago

Phergus commented 5 years ago

Creating new light sources in Campaign Properties for environmental lights that are only used once or can vary in size/color/shape is both tedious and results in a cluttered Light menu.

Being able to create lights dynamically via macro would make it much easier to handle situations like light spells in games where the size and shape are chosen at the time of casting or environmental effects such as when someone sets a large pile of lumber on fire.

Dynamic light sources would not be added to the campaign properties upon creation. (Optional?)

They would be preserved in the saved campaign such that tokens/objects/stamps would carry them through campaign save/load operations.

Suggested functions:

Other possible functions

Existing Light functions

Begin discussion.

Azhrei commented 5 years ago

Overall, I like this idea. :)

Being able to create lights dynamically via macro would make it much easier to handle situations like light spells in games where the size and shape are chosen at the time of casting or environmental effects such as when someone sets a large pile of lumber on fire.

Dynamic light sources would not be added to the campaign properties upon creation. (Optional?)

They would be preserved in the saved campaign such that tokens/objects/stamps would carry them through campaign save/load operations.

Are they preserved only when there's a token that has them turned on, so that if no tokens are using them, they would be purged from the campaign?

Suggested functions:

  • createDynamicLight("name", "definition string" [, "tokenid"])
  • getDynamicLights(["tokenid" [, "delim"]])
  • removeDynamicLight("name"|"all")

Are strings the way to go in defining a light? There's an obvious connection between using a given string in the campaign properties and the same string in a macro, but wouldn't a JSON object of properties make more sense?

Other possible functions

  • modifyDynamicLight("existing name", "new definition") - or maybe create on an existing dynamic light just overwrites?

Existing Light functions

  • setLight("Dynamic", "name", 0|1) - passed a type of "Dynamic" and a dynamic light name applies that to current token
  • getLights() - works as normal with Dynamic lights having type "Dynamic"

Is there any value in getLights() returning the number of tokens (or the tokenids of tokens) currently using a particular light definition? (I can't think of any off the top of my head.)

(Looks like setLight() should take an optional tokenid like most functions do.)

Jaggeroth commented 5 years ago

I wonder whether you really need the ability to create lights on the fly. Wouldn't you know them all up front?

Isn't all you really need the ability to add lights and be able to turn them on and off?

Phergus commented 5 years ago

Are they preserved only when there's a token that has them turned on, so that if no tokens are using them, they would be purged from the campaign?

My first thought is no as they will be primarily created via campaign or token macros and so will be recreated if needed.

Are strings the way to go in defining a light? There's an obvious connection between using a given string in the campaign properties and the same string in a macro, but wouldn't a JSON object of properties make more sense?

Yes, it probably would. I was too lazy to go look up in the code what the JSON would need to comprised of.

Is there any value in getLights() returning the number of tokens (or the tokenids of tokens) currently using a particular light definition? (I can't think of any off the top of my head.)

Seems like something it should do. It is an existing function so that would be a change to it.

Possible use case: If you have a bunch of temporary light sources like, say burning oil from tossed oil flask, I could see managing those via macro so getting a list of those tokens would be nice.

(Looks like setLight() should take an optional tokenid like most functions do.)

I thought the same thing when I went and looked at the function.

JamzTheMan commented 5 years ago

Custom light radius distance alone would reduce a lot of light definitions... will keep this in mind

Phergus commented 5 years ago

I wonder whether you really need the ability to create lights on the fly. Wouldn't you know them all up front?

I never know up front what my players are going to set fire to. Giant wood piles, towers, trolls, wagons, haystacks, buildings, each other... The list goes on and on.

I do have pre-defined Light sources for common things like candle, torch, lanterns, small campfire, large campfire, brazier, sun, moon, as well as 18 different light spell variations which doesn't begin to cover the actual possibilities. Not to mention I don't want/need the players seeing a list of 50 or so light definitions.

Azhrei commented 5 years ago

There's something to be said for an onCampaignLoad macro being able to properly set everything needed in the campaign properties for the framework to function.

Phergus commented 5 years ago

Custom light radius distance alone would reduce a lot of light definitions... will keep this in mind

Yeah. Just the ability to change the radius on the fly would be a huge improvement.

Phergus commented 5 years ago

Can someone put the appropriate labels and such on this?

Jaggeroth commented 5 years ago

I never know up front what my players are going to set fire to. Giant wood piles, towers, trolls, wagons, haystacks, buildings, each other... The list goes on and on.

I do have pre-defined Light sources for common things like candle, torch, lanterns, small campfire, large campfire, brazier, sun, moon, as well as 18 different light spell variations which doesn't begin to cover the actual possibilities. Not to mention I don't want/need the players seeing a list of 50 or so light definitions.

Even so, isn't it harder to create on the fly light source by code? I am struggling to think of an example. Although I suppose being able to vary the distance could be useful.

kwvanderlinde commented 10 months ago

I've nearly got something put together for this. The idea is that macro functions can define light sources on tokens rather than on the campaign, but otherwise they function very similarly to campaign light sources. So not "dynamic" per se, but this functionality is enough for a framework to dynamically add/remove specialized light sources on specific tokens. The right-click menu will also show the token's light sources to allow for easy toggling by the user.

New macro functions:

The functions createTokenLightSource(), getTokenLightSource(), and getTokenLightSources() use this JSON representation for a light source:

{
    "name": "Torch - 20",
    "type": "NORMAL",
    "scale": false,
    "lights": [
        {"shape": "CIRCLE", "range": 20, "lumens": 100},
        {"shape": "CIRCLE", "range": 40, "color": "#000000", "lumens": 50}
    ]
}

Other fields are also available for parity with Campaign Properties, e.g., "arc" and "offset" for cones or "gmOnly"/"ownerOnly" for auras.

Changes to existing functions:

E.g., to check if a token has any light sources, this will still work:

[h: hasLightSource("*", "*")]

To check specifically whether any token light sources are attached:

[h: hasLightSource("\$token", "*")]

And to attach a token light source to the current token:

[h: setLight("\$token", "some custom light", 1)]

Feedback on names would be very welcome. Well, feedback on everything, but names are the hard part. Because this isn't really "dynamic lights" I didn't go for that naming. But I also am aware that calling them "token light sources" does little to distinguish these from the concept of a token's attached light sources.

Pmofmalasia commented 10 months ago

Very excited for this, I'll definitely be using it when it's implemented! Especially like being able to set the lights via JSON instead of being string-based only. Would be great if a similar method could also be done for vision.

As for the naming, I think something like "Individual" or "Unique" or something along those lines would be good, since their defining characteristic is less that they're on a token and more that they're separate from the campaign lights. Could probably do better than one of those two, but something along those lines.

kwvanderlinde commented 10 months ago

Very excited for this, I'll definitely be using it when it's implemented! Especially like being able to set the lights via JSON instead of being string-based only.

:eyes: Just so it's clear, I'm not planning to support plain strings. Although it would be more terse than JSON, our little light language has more complexity than it's worth IMO, so I'd rather not use it here. Even in the Campaign Properties dialog that syntax for lights isn't going to stick around forever (assuming we can figure out the UI for it).

Would be great if a similar method could also be done for vision.

That would a natural next step.

As for the naming, I think something like "Individual" or "Unique" or something along those lines would be good, since their defining characteristic is less that they're on a token and more that they're separate from the campaign lights. Could probably do better than one of those two, but something along those lines.

Yes, I do like "unique"! It contrasts so well with the generic nature of campaign lights. Thank you for the suggestion :smile:

Pmofmalasia commented 10 months ago

Oh, yes, fully agreed on ditching the strings. Much clunkier to keep those around than just using JSON (or inputs for the dialog).

kwvanderlinde commented 8 months ago

I'm considering reverting these changes for now (plus those for #3087) as I'm concerned they will set in stone a few unnatural aspects of lights (mixups with auras, the way grids work right now, etc).

cwisniew commented 8 months ago

I'm considering reverting these changes for now (plus those for #3087) as I'm concerned they will set in stone a few unnatural aspects of lights (mixups with auras, the way grids work right now, etc).

If it helped with your considerations :) I think it's better to remove it before it's used by people to fix what you want to fix with lights and Auras, there are already too many things hard to fix without breaking macros :)