Jagusti / fvtt-wfrp4e-gmtoolkit

Utility module with tweaks, enhancements and macros to help GMs with game management when running Warhammer Fantasy Roleplay (4e) sessions in Foundry Virtual Tabletop.
MIT License
15 stars 9 forks source link

Fix group advantage #262

Closed Forien closed 5 months ago

Forien commented 6 months ago

Type of change

Checklist:

Related Issue(s)

Description

Motivation and context

Checking Document.ownership[game.user.id] is unsafe, as it may (and will) return undefined for GM, if GM is not explicit Owner of a Document.

For example:

There are 2 users in the world:

  1. GM with aDepqlKkeOa13DWo id
  2. User with JYO5JplQ4zaynRJ4 id

There is an Actor with the following ownership structure:

ownership = {
    "default": 1,
    "JYO5JplQ4zaynRJ4": 3
}

Current way of checking ownership would return undefined for GM, even though GM is always an implicit Owner of any and every Document.

Which is why using Document.isOwner getter is superior, and in my opinion, the only proper way of checking the ownership for current user.

Why didn't it work?

Actors created by Players (for example using Chargen if they have permission) do not have GM as an explicit Owner. Similarly, Actors that can be created programatically may not have GM as explicit Owner neither.

Summary of changes

Replaced every occurrence of `.ownership[game.user.id]` with `.isOwner` ### How has this been tested?

Ran Opposed Tests, using "Group Advantage" ruleset with Actors created by User as Attackers.

Development / Testing Environment

Btw, I spend more time filing this form than making PR lol 😂