Roll20 uses cookies to improve your experience on our site. Cookies enable you to enjoy certain features, social sharing functionality, and tailor message and display ads to your interests on our site and others. They also help us understand how our site is being used. By continuing to use our site, you consent to our use of cookies. Update your cookie preferences .
×
Create a free account

[PF] Sheet update discussion

1448499174
chris b.
Pro
Sheet Author
API Scripter
new update added to the beta site above and to github i did a pull request I found this&nbsp; <a href="https://help.github.com/articles/resolving-a-merge" rel="nofollow">https://help.github.com/articles/resolving-a-merge</a>... I think i could merge it to my master thenit might be easier to merge to yours, but not sure. this has the stuff you committed a few days ago, plus my fixes for the issues you found plus NPC page plus I added the saves to items updated via javascript all macros work again, i guess i cut and pasted something wrong before.
1448499850
Sam
Pro
Sheet Author
When I get home I'll get Intellij up and running again. It has a nice built in merge conflict resolver. That should make it easier.
1448500077
vÍnce
Pro
Sheet Author
What's funny about the merge conflict is that there aren't any... &nbsp;:-) I look over your PR it's fine. &nbsp;Perhaps your're right. &nbsp;PR to your fork of my fork (this is comical) then send me a PR. &nbsp;Thanks Chris.&nbsp;
1448510649
chris b.
Pro
Sheet Author
API Scripter
ive been trying for the past hour to merge my javascript-update branch to my master branch, and it keeps saying it is done, but the master file never changes. I am pretty new to git so ... i am reading the docs on what to do. I did manage to generate a file of changes and I updated it .&nbsp; i am giving up for now these are the correct files <a href="https://github.com/plutosdad/roll20-character-sheets/tree/javascript-updates/Pathfinder-Neceros" rel="nofollow">https://github.com/plutosdad/roll20-character-sheets/tree/javascript-updates/Pathfinder-Neceros</a>
1448519722
vÍnce
Pro
Sheet Author
Your PR to me still said it had merge conflicts..., but I had no problem using the desktop gui and merging your branch to Roll20/master <a href="https://github.com/Roll20/roll20-character-sheets/" rel="nofollow">https://github.com/Roll20/roll20-character-sheets/</a>...
1448727090
chris b.
Pro
Sheet Author
API Scripter
new commit, &nbsp;now in plutosdad master branch. mainly cleanup and improvements, only to javascript at bottom not to html , only "new" thing is fractional class advancement supported now. next Im trying to incorporate the Extended Expressions API module to create rules engine to parse macro fields&nbsp; <a href="https://wiki.roll20.net/Script:Extended_Expression" rel="nofollow">https://wiki.roll20.net/Script:Extended_Expression</a>... If possible, that will get us back to how the sheet worked before.
1448742215
vÍnce
Pro
Sheet Author
New PR #1162 submitted <a href="https://github.com/Roll20/roll20-character-sheets/" rel="nofollow">https://github.com/Roll20/roll20-character-sheets/</a>... So I made a PR from your fork's master from my fork's master back to me..., still has merge conflicts. &nbsp;Go figure. &nbsp;All I do is look at the PR locally and merge it to my fork and submit a new PR. &nbsp;No changes necessary. &nbsp;I'm thinking this is just part of the "peer review" of a project with git. &nbsp;It's fine. &nbsp;If we both made PR's to the master branch for roll20 character sheets, I suppose whomever the repo manager is would have to deal with it similarly. &nbsp;We're just handling before it gets there which is probably a good thing. &nbsp; Been side-tracked by another sheet (don't ask.) &nbsp;Thanks for the work Chris.
1448826967
chris b.
Pro
Sheet Author
API Scripter
I think if I do a push from your master into mine then merge them it might eliminate this. I finally got the macro parsing working for fields like the buff macro text, and other areas.The only issue is I can't parse kh1 kl1 or functions that appear at the end of a set of brackets. .. probably the best way of doing it is to make the macros evaluate to hidden fields then use those hidden fields (or maybe we can put the fields next to the macro text fields)&nbsp; not sure if i can get it in git tonight probably i can put in the buffs at least for testing, the others i have not worked on, since to parse it requires yet another round trip to the server.
1448869515
Sam
Pro
Sheet Author
How do custom attack macros currently stand? I customize them a lot due to how I do my characters so those still need to be a thing.
1448878734

Edited 1448878806
vÍnce
Pro
Sheet Author
They should be fine, but they'll have to be edited to include " $ " for each repeating attribute used. ie @{repeating_foo_X...} becomes&nbsp;@{repeating_foo_$X...} where X is a row number. &nbsp;You should try some of your macros out on the Dev beta. &nbsp;I believe buffs are an issue since the macro text causes problems when processed by the sheet worker scripts.
1448894961

Edited 1448895325
chris b.
Pro
Sheet Author
API Scripter
I actually put the code in beta that allows the buff macro text to be parsed, i only tested simple ones, but you can try that out.&nbsp; edit: I also just tested and if you have, say, @{level} in the buffx_xxx-macro-text , then update level in the classes, the buff will get updated as well, super.
1448896703
vÍnce
Pro
Sheet Author
Like magic Chris.
1448920223

Edited 1448920235
chris b.
Pro
Sheet Author
API Scripter
I think the problem was I was not merging Vince's commits back into my repo. Did it and now it says the pull request can be automatically merged. so Vince you can try it again.&nbsp; I did the following: git merge upstream/master git status #here I looked in the file, saw one conflict (not really but I took Vince's rows in case) git add Pathfinder-Neceros/pathfinder-neceros.html git commit git push origin master sigh , TFS is so much easier with its nice GUI, :) of course, it's harder to script updates for TFS too. And TFS Branches are no different than Forks
1448926186
vÍnce
Pro
Sheet Author
No merge conflicts! &nbsp;:-) &nbsp;New PR <a href="https://github.com/Roll20/roll20-character-sheets/" rel="nofollow">https://github.com/Roll20/roll20-character-sheets/</a>...
1449023325

Edited 1449023350
chris b.
Pro
Sheet Author
API Scripter
New update commited and in beta site. The damage and attack mods in the repeating weapons now allow macros again. I put a field to the right of each that shows the evaluated value of the macro. Is there a way to get more than 6 players in the beta? I can't find the limit, there used to be one somewhere on the campaign settings. Current bugs: 1. buff total at upper right is off 2. "recalc sheet" broken 3. i just realized I did not update NPC repeating weapon section to match. so i may need to do that. Other than fixing those bugs, what should be the next priority? I am pretty busy at work so need to slow down and &nbsp;just keep it working. one fix:&nbsp;Size modifier for skill can now be updated to not require a second dropdown. that may be easy. Other than that: The conversation in the dev thread about updating macros might be the important issue, but it sounds like Aaron is thinking of doing something.
1449032164
vÍnce
Pro
Sheet Author
That's good news Chris. &nbsp;NPC side has always been the snafu for update errors. Easy to forget about the duplicated code. &nbsp;There shouldn't be any limit to the number of campaign player's AFAIK. &nbsp;Maybe we should post the Dev Beta link to the regular PF thread where some people may be following/subscribing. &nbsp;I bet we'll get an increase in testers. &nbsp; I'll try to get back on the sheet and make some updates. &nbsp;Removing presets(button bars) and making "/w gm" an option for NPC rolls(maybe even expand this as optional for all rolls?).
1449191837

Edited 1449193088
vÍnce
Pro
Sheet Author
Apologies for not jumping in on the sheet's code and helping out with beta testing.... life stuff. &nbsp;Should be able to do more after this weekend. It looks like ACP get's applied as a condition for melee/ranged/cmd when you select armor proficiency "no", but if you set it back to "yes", ACP is still applied. Made a new character and&nbsp;Fort and Flat-Footed are not being auto-calculated.&nbsp; buff-Will-total is not being calculated. (it appears to after recalc) In Class Information: it looks like you have to enter a number (even if it's "0") for each field in order to get a total at the bottom. &nbsp;Thought you could just tab through the fields and the default would be used? Skills tab: Background Skills remaining only shows a formula.
1449193903

Edited 1449193946
vÍnce
Pro
Sheet Author
NOT a PRIORITY: Is it possible to use Brian's code example here: <a href="https://app.roll20.net/forum/post/2660799/sheet-wo" rel="nofollow">https://app.roll20.net/forum/post/2660799/sheet-wo</a>... on the @size attribute to not only set the appropriate mod as it applies to AC and BAB but also create a second attribute mod as it applies to fly and stealth? &nbsp;I had to add query to both skills to account for size, but I thought maybe we could get a "2 for 1" attribute set with js now...?
1449194487
chris b.
Pro
Sheet Author
API Scripter
Yes, I was starting the skills and that's one of the things I was going to do. The js makes it easier
1449329214

Edited 1449329837
chris b.
Pro
Sheet Author
API Scripter
I will be travelling between thurs 12/10 to wed 12/16 so probably unable to work on the sheets AT ALL during that time. I suspect they will put out the update any day now so my first priority probably should be getting the "recalc all" button working and .. I think adding a field for Version, and putting a call to recalc all in the new "on sheet open" event they added, which will check the version and call different versions of recalc all. unfortunately last week I worked on fun stuff instead: Note: i committed most of this but have not done a pull request yet, since it is not nearly finished: (the javascript is, the html is not) I have tested and got the size to skill stuff working for fly and stealth (much easier to do this in javascript since we can use if.. then) but the entire skill grid is a lot of html to go through and fix and make sure is working so the javascript changes were probably actually easier and less tedious than updating the skill grid will be. there are now 4 size attributes: size (same as now, used for Attack and AC) cmd-size (-1 * size, used by CMD and CMB) skill_size (0,2,4,6,8 , used by Fly) skill_size_double (skill_size * 2 , used by Stealth) i named them generically just in case.&nbsp; I also tested updates to the skill grid successfully for a few skills, so that whether to apply ACP, Size, or doubleSize can be configurable, using the names: "skillname-acp" or "skillname-size" . Since that is a ton of room i think maybe putting it in the config tab would be better, players won't normally have to change it, or maybe ever, Or ... maybe that is too much and will only result in slowness and more configuration than necessary? Because i can just as easily hardcode whether to apply ACP, skillSize, or 2* skillSize which would be faster . though really anyone can do this part:&nbsp; basically for the skill grid we have to : 1. only for the skill grid sections,&nbsp;search and replace : &nbsp; &nbsp; &nbsp; &nbsp;disabled with &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;class="sheet-calc" readonly="readonly" 2. search and replace: (for the field displaying the attribute modifier) &nbsp; &nbsp; &nbsp;-display with &nbsp; &nbsp; &nbsp;-mod&nbsp; 3. add a skill section&nbsp;in the config page, and for each skill using acp add the checkbox: &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; foreach skill using acp: &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;name="attr_skill-acp" value="1" checked="checked" &nbsp; &nbsp; for fly and stealth also add: (note value changes) &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;FLY uses the skill_size:&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;name="attr_fly-size" value="1" check="checked" &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;STEALTH uses 2* skill_size: &nbsp; &nbsp; name="attr_stealth-size" value="2" check="checked" &nbsp; we can add others if we want for completion, that are default unchecked, but code will at least run without them the above is mandatory, the rest is not but is nice: 4. ideally replace the value="macro" with value="0" for those disabled rows 5. update class column to be 4em wide, and change header 6.&nbsp;remove the acp column #4-6 is&nbsp;because i changed the "class" column to display a combo of these 3:&nbsp;"classSkilmod / acpmod / sizemod" , so for the Fly entry for a Large character wearing no armor, for whom Fly is a class skill, would look like: "3/0/-1" that widens one column a little but the acp column and "+" column take up more space than we add. so we get some whitespace back. I did that because I wanted to add size, but another column was too much, especially for just 2 skills. so i combined all 3 into one since they are readonly anyway.(also I was thinking of adding those 2 checkboxes to the grid and wanted to get space back but .. whether there or in config tab is whatever you think is best) i wrote the header as "Class/AC/Size" but that's too wide, not sure what to put that will identify
1449415490
chris b.
Pro
Sheet Author
API Scripter
New update and pull request it's in the beta site too fixes: Fixed recalc all function added version number to sheet Added text to top to remind user to hit recalc all narrowed down bugs in repeating weapons (note at top of sheet), the dropdowns work, but static references to other items on sheet not working (so dmg-buff, attk-effect-total and dmg-effect-total don't automatically force the repeating weapon to update) ensured none of my test html for skills got in sheet so i think it's pretty good and stable.
1449449224
vÍnce
Pro
Sheet Author
Been out of town. &nbsp;Will have a look at the skill section. &nbsp;PR merged.
1449527048

Edited 1449527186
chris b.
Pro
Sheet Author
API Scripter
Fixes in in the repeating section. &nbsp;(ACP to hit is broken still, not sure why, i keep fixing it . argh) This is the most stable update now except ACP. I saws in email they are merging a ton of pull requests for the character sheets now, so maybe they are going to go live. if so, well i guess we need to do the pull request of our sheet to get it in . But if the old sheet works in dev, then probably we can leave it. I saw he pulled in the fixes you made a few weeks ago just now.
1449533100
vÍnce
Pro
Sheet Author
Just got home from work... &nbsp;it looks like they merged all the PR's... &nbsp;really wish we would have had at least a 24hr notice. &nbsp;I just merged your pr to my fork. &nbsp;I'll add my changes and make a new PR to Roll20's repo.
1449533775

Edited 1449540322
vÍnce
Pro
Sheet Author
I'll update with my commits later({{subtitle}} added to macro text, other small changes, will have a go at updating skills as per above), but this has your work Chris. &nbsp;Maybe we'll get a back to back merge by Ryan... <a href="https://github.com/Roll20/roll20-character-sheets/pull/1176" rel="nofollow">https://github.com/Roll20/roll20-character-sheets/pull/1176</a> added NPC whisper option on Config page added "Type" key/property to pf_attack
1449540698
chris b.
Pro
Sheet Author
API Scripter
Cool. Don't submit any of the skill update changes I refer to above, I did not put the event handlers in yet, so the skills should stay as disabled calculated fields Yes it was odd to suddenly get so many emails of about the merges. It is weird to have so many changes and then if there are bugs people find .. It's not like we are employees. Not sure really what will happen with support as more and more users sign up.
1449556443

Edited 1449564514
vÍnce
Pro
Sheet Author
<a href="https://github.com/Roll20/roll20-character-sheets/" rel="nofollow">https://github.com/Roll20/roll20-character-sheets/</a>... added PC whisper option to config tab.&nbsp; added level, range to NPC spell-like abilities and spells and added to macro text.&nbsp; added {{subtitle}} into sheet macros.&nbsp; removed readonly from buff checkboxes and re-numbered rows.&nbsp; reduced size of clickable area for attack repeating item show/hide (was overlapping button roll). Armor Proficiency still seems to only work "one-way". &nbsp;Set to "No" and ACP is applied to Melee/Ranged/CMB as a Condition. &nbsp;Set back to "Yes" ACP is still applied.
1449572694

Edited 1449587368
chris b.
Pro
Sheet Author
API Scripter
Weird that buff checkboxes &nbsp;still worked I'll try to get acp fixed today, at least no one is likely to mark their armor non proficient&nbsp;
1449603557
chris b.
Pro
Sheet Author
API Scripter
I fixed acp tohit, and found another small bug or two.&nbsp; I added some more info to the top announcement. ideally we'd have a way to collapse it, at least making sure people read it once. checked in.
1449603587
chris b.
Pro
Sheet Author
API Scripter
I think if you commit it will automatically be added to your pull request, you don't have to close and make a new one. but we'll see.
1449640028
vÍnce
Pro
Sheet Author
<a href="https://github.com/Roll20/roll20-character-sheets/" rel="nofollow">https://github.com/Roll20/roll20-character-sheets/</a>... button bars have been removed. RIP
1449702799

Edited 1449702930
vÍnce
Pro
Sheet Author
Updated the Attention text and gave some formatting similar to the footer. <a href="https://github.com/Roll20/roll20-character-sheets/" rel="nofollow">https://github.com/Roll20/roll20-character-sheets/</a>... Issue : &nbsp;I don't think Buff DMG or Attack/Damage Effects are being applied to repeating weapons at all. &nbsp;I tried toggling values within the repeating weapon. &nbsp;I know you recently change the attribute names for effects and buff totals. ( &nbsp;@{attk-effect-total} to&nbsp;@{attk-effect-total-copy}. &nbsp;) example taken from repeating weapon attributes &lt;input type="number" name="attr_attk-effect-total-copy" title="@{repeating_weapon_$X_attk-effect-total-copy}" value="0" style="width:40px;" class="sheet-calc" readonly="readonly" &nbsp;/&gt; should the value be "@{attk-effect-total}" instead of "0"...?
1449704678

Edited 1449704699
chris b.
Pro
Sheet Author
API Scripter
ack, ok i found it, I left the line&nbsp;if (ids.hasOwnProperty(id)) in by accident, that if statement and the { and } need to be commented out or deleted. i will check in a new version .. i can't merge my changes with yours because yours are not in the vinceroll20/master but i added it just now. the only differences are in the code so you can just delete the old code section and paste the new one in at the bottom and leave the html alone.
1449705061

Edited 1449705145
chris b.
Pro
Sheet Author
API Scripter
One thing I notice now: the Total column for Defenses should be widened. or change to text, but im not sure changing to text will cause any issues. I don't think so. Actually: changing the "Conditions" heading to "Cond" might be enough i am going out of town in the morning and won't be back till next wednesday. i should have some internet access but not much so i want to do most testing i can now and not make more changes&nbsp;
1449706628
vÍnce
Pro
Sheet Author
Added your patch. <a href="https://github.com/Roll20/roll20-character-sheets/" rel="nofollow">https://github.com/Roll20/roll20-character-sheets/</a>... The code from PR#1182 will have "our" latest changes if you want to grab the raw. &nbsp;I'll try and push my local copy to my fork also. I must be doing something wrong with git. &nbsp;I update locally, test online, then I save my changes to my local repo. The git gui detects the change when I open it and I "commit to master". &nbsp;I would think this would also update my fork since my local mirrors my online repo. I'll have a look over the issues you mentioned above and make some adjustments. &nbsp;Be safe on your trip. &nbsp;This may&nbsp;get interesting once the update hits the main servers... &nbsp;:-P
1449708630
chris b.
Pro
Sheet Author
API Scripter
thanks oops found this: at the top announcement we can delete the one about "updating buff dmg, attack, dmg" if you confirm they are working. &lt;li&gt;Updating Buff DMG, and/or Attack and Damage Effects will not automatically update repeating weapons. You must either toggle them on/off or go to the weapon and change a value and then change it back to force an update.&lt;/li&gt;
1449711504

Edited 1449711735
vÍnce
Pro
Sheet Author
Yep. &nbsp;I'll take that out and update... done.
1449728427

Edited 1449733398
vÍnce
Pro
Sheet Author
Update: Fly and Stealth Skill checks no longer need/have query adjusted various column headers text for layout adjusted layout of Details tab to make room for the various Size modifiers mirrored PC/NPC repeating attack and added readonly to some of the NPC auto-calc fields
1450022032

Edited 1450022192
chris b.
Pro
Sheet Author
API Scripter
Cool. I did do a lot on a test page for skills and it was working well, i modifird all the skill rows, it is not committed yet since I have to convert the temp column to a condition modifier, to account for changes due to conditions. I am wondering if moving the macro column to the config page would help make the skill page easier to read. the one thing I don't like is when applying conditions that eliminate dex to AC and it set the mod due to ability to equal the flat footed modifier (usually 0 but in case of uncanny dodge and similar "do not lose dex") abilities, but the number displayed in the ability mod field does not change, so players may not realize why the AC is suddenly lowered. I suppose I could add a " display " field.&nbsp; I am basically using the "ff dex ability" drop down as a switch to determine if a character does not lose dex to AC , if it is not 0/"none" then I assume the char has a "do not lose dex to AC" ability. And then no matter what condition it does not change the AC, except for pinned, since the char is immobile even uncanny dodge says immobility causes loss of dex, I figure any other ability is the same. Since I read in the FAQ for int replacing dex if you lose dex to AC you also lose int to AC, I always apply the condition regardless of what the ability is. But making sure dex to AC was reset for conditions slowed down the sheet some, sigh.&nbsp; I will have time starting Thurs to look at it again.&nbsp;
1450026929
vÍnce
Pro
Sheet Author
That's it, our sheet just needs to go landscape. &nbsp;;-)