mr-ice / maptool-macros

place for maptool macros
2 stars 3 forks source link

Lib:Open5e needs to use setProperty #331

Open trey-kirk-sp opened 3 years ago

trey-kirk-sp commented 3 years ago

IIRC, I did these intentionally at the time. But using var names that clash with campaign props can be a disaster. The following needs to be converted to use setProperty

 {
    "Lib:Open5e:o5e_Monster_refreshMonster_v15:CR": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Monster_refreshMonster_v15:Proficiency": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:AC": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Walk": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Swim": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Burrow": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Fly": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Climb": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Strength": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Dexterity": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Constitution": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Intelligence": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Wisdom": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Charisma": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Resistances": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Immunities": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Languages": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:HitDice": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:HP": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:MaxHP": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:CR": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_Monster_applyProperties:Proficiency": "FAILED! Matches campaign property",
    "Lib:Open5e:o5e_Token_applySenses:Senses": "FAILED! Matches campaign property"
}
sachindavra commented 3 years ago

Hi, I would like to work on this issue. Could you tell me more about this?

trey-kirk-sp commented 3 years ago

Hi @sachindavra. Well, sure, we'll gladly accept the help!

What this defect is dealing with is one of our Token Libraries, Lib:Open5e, is using a few macros that are declaring variable names that match campaign properties. For example, in Lib-Open5e/o5e_Token_applySenses.command you'll find the following expression:

[h, if (matchCount > 0), code: {
    [Senses = getGroup (matchId, 1, 1)]
}; {
    [Senses = sensesValue]
}]

"Senses" is a campaign property. While we're intentionally setting this variable because it is a campaign property, a better practice for us is to instead use 'setProperty'. So this macro would need to be changed to:

[h, if (matchCount > 0), code: {
    [setProperty ("Senses", getGroup (matchId, 1, 1))]
}; {
    [setProperty ("Senses", sensesValue)]
}]

In the original bug description, my reporter generates the following example: "Lib:Open5e:o5e_Monster_refreshMonster_v15:CR" Which simply translates to TokenLibrary:Macro name:Variable name. These are all in the Lib:Open5e token library (/Lib-Open5e in our source control). Each macro is a .command file within that directory. You can fork the repository and push a PR with your changes, if you're still game to do this.

sachindavra commented 3 years ago

Is this written in python?

havoclad commented 3 years ago

The majority of our code is written in Maptool's internal macro language. Our build tools are mostly python though.

sachindavra commented 3 years ago

Apology, I don't have knowledge about Maptool. If there is any other way to contribute in this project, kindly let me know.

mr-ice commented 3 years ago

Hi @sachindavra ,

Thanks for your interest.

The build tools are mostly python as @havoclad said. I have a small amount of Java in there as well to interface with the MapTool objects natively (MapTool is a Java project). We use python3 lxml.objectify to pull apart the objects that come out of MapTool's various save functions and prepare them for checking into github. Natively the MapTool objects are a zip archive of an xstring (xml) representation of the objects themselves. The build tools are there to allow MapTool objects to be composed from github artifacts, which gives us finer control over the changes in github compared to trying to track the zip archives themselves. Tests for the build tools are written in pytest and behave.

I haven't managed to tag any of the build tools issues as good first issue, so some digging on your part might be required.

To get started on either the build tools or the maptool objects you would need both docker and MapTool installed. The latter is relatively easy, head over to their repo and check out their releases. Docker desktop is a fine place to start, and just make sure you can start a linux container (like alpine).

@trey-kirk-sp tagged this one as a good first issue because it's nearly a search and replace, you would need to get familiar with the objects and terminology along the way, and you'd need to be able to build and extract so it exercises a lot of the machinery. The work is in the 'nearly'.

If you were comfortable in python/pytest/behave and just wanted to look around and offer suggestions that would also be a way to get some familiarity with the project. PRs are always welcome.

sachindavra commented 3 years ago

Hi @mr-ice ,

Thanks for your reply. I will start digging to find good first issues. I would like to start with build tools then as i can build docker machine. I will check later repo. In case any help is required, whom should i reach. Do we have any slack channel or communication method rather than comments of issue.

It will be helpful if you could give me the link of latter repo.

trey-kirk-sp commented 3 years ago

Updated the property clash detector to normalize values and found more:

{
    "Lib:DnDBeyond:Spend Hit Dice:CLASSES": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:Spend Hit Dice:HITDICE": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:dndb_buildBasicToon:INITIATIVE": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:dndb_getBasic:HITDICE": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:dndb_getBasic:RESISTANCES": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:dndb_getBasic:IMMUNITIES": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:dndb_getHitPoints:HITDICE": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:dndb_getHitPoints:MAXHP": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:dndb_getInitiative:PROFICIENCY": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:dndb_getProficiencyBonus:PROFICIENCY": "FAILED! Matches campaign property",
    "Lib:DnDBeyond:dndb_getSavingThrow:DEATHSAVE": "FAILED! Matches campaign property"
}
{"Lib:DnD5e:dnd5e_SavedAttacks_applyRolledExpression:AC":"FAILED! Matches campaign property"}
{
    "Lib:DnD5e-CharacterSheet:dnd5e_CharSheet_getAttacksHTML:ATTACKJSON": "FAILED! Matches campaign property",
    "Lib:DnD5e-CharacterSheet:dnd5e_CharSheet_rollAttack:ATTACKJSON": "FAILED! Matches campaign property",
    "Lib:DnD5e-CharacterSheet:dnd5e_CharSheet_rollAttackOnly:ATTACKJSON": "FAILED! Matches campaign property",
    "Lib:DnD5e-CharacterSheet:dnd5e_CharSheet_rollDamageOnly:ATTACKJSON": "FAILED! Matches campaign property"
}

Lib:DarkerDungeon, Monster-Supplement, PartyPanel, & MultiSelect all currently pass.