API request: Pass the object id when calling `CONFIG.Canvas.losBackend.create` #6111

Closed aaclayton closed 2 years ago

Originally in GitLab by @caewok

Feature Summary

CONFIG.Canvas.losBackend creates the line-of-sight polygon for sight, sound, and light. It is fairly straightforward to extend the algorithm used by extending the underlying class and then setting the extension to CONFIG.Canvas.losBackend.

One conceivable way a module developer might want to extend the line-of-sight calculation is to allow it to handle special types of objects. For example: lights with non-circular radius, lights or sound that penetrate through certain wall types, a token with special vision features, etc. The catch is that the line-of-sight calculation needs to know what object it is calculating los for. That is so a user could then mark a special attribute for a light, token, or sound, that would in turn change how the los calculation is done.

To accomplish this requires a very minor set of tweaks:

Modify the _initialize method in the LightSource Class to pass the object id:

    if ( !this.data.walls ) this.los = new PIXI.Circle(origin.x, origin.y, this.radius);
    else this.los = CONFIG.Canvas.losBackend.create(origin, {
      type: "light",
      angle: this.data.angle,
      density: 60,
      radius: this.radius,
      rotation: this.data.rotation,
      id: this.object.id // pass the Light id through to the los calculation.
    });

Personally, I would also move this check into the relevant losBackend for consistency:

if ( !this.data.walls ) this.los = new PIXI.Circle(origin.x, origin.y, this.radius);

This check assumes you always want a circular los if you have no walls and so is better handled by the underlying losBackend. For example, a modification to the losBackend that creates square lights would fail when there are no walls because of this check. (ClockwiseSweep, and I suspect the others, already handle a full circle polygon when walls exist on the scene, so dropping this check should not break anything. ClockwiseSweep is also very fast when no walls are present, so I would expect minimal loss of performance in this edge case.)

Same modification to the _initialize method in the SoundSource Class:

if ( !this.data.walls ) this.los = new PIXI.Circle(this.data.x, this.data.y, this.data.radius);
  else this.los = CONFIG.Canvas.losBackend.create({x: this.data.x, y: this.data.y}, {
    type: "sound",
    radius: this.data.radius,
    density: 60,
    id: this.object.id // pass the Sound id through to the los calculation.
  });

Also same comment about moving the !this.data.walls to the underlying losBackend.

And finally, same modification to the _initialize method in the VisionSource Class:

this.los = CONFIG.Canvas.losBackend.create(origin, {
  type: "sight",
  angle: this.data.angle,
  rotation: this.data.rotation,
  id: this.id // pass the VisionSource id through to the los calculation
});

Here, I am not sure if the VisionSource id is equivalent to the token id. Or like the others, it might be object.id. I would expect vision features to be tied to token, so I would expect that token id would be the useful piece of information to pass here.

User Experience

Users have no direct impact, but indirectly benefit:

This minor change will more easily allow base Foundry, system developers, and module developers to tie special vision/light/sound features to the los calculation.
Module developers will not need to override the _initialize methods to change a single item, which should make for less problematic module interactions.

Priority/Importance

Fairly unimportant relative to everything else here, but it could make a big difference for certain Foundry developers and is (I think) an easy change.

I would add that this is probably a good time to make this tweak. v9 takes advantage of the CONFIG.Canvas.losBackend setup and added several los algorithms, so this would be one way to polish that API by passing through a bit more data to handle potential future use cases.

Originally in GitLab by @Fyorl

An optional source property has been added to the PointSourcePolygonConfig object now, which contains a reference to the PointSource that created it, if any.

Originally in GitLab by @caewok

Looks like _initialize could have an object that does not (yet) have an id. For example, when dragging a light around the canvas or previewing a light change, I think. So might be better to just pass this.object instead of this.object.id.

foundryvtt / foundryvtt