Skip to content
This repository was archived by the owner on Jul 25, 2021. It is now read-only.

Patch 12.3#18

Merged
omniraptorr merged 18 commits intomasterfrom
patch_12.3
Apr 29, 2020
Merged

Patch 12.3#18
omniraptorr merged 18 commits intomasterfrom
patch_12.3

Conversation

@omniraptorr
Copy link

closes #14 . one thing i'm curious about is the pdf highlight function, i never got it to work. otherwise please leave any feedback etc.

Copy link
Member

@Benjamin-Dobell Benjamin-Dobell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, there's a lot of stuff here! Mostly just nitpicks.

@Benjamin-Dobell
Copy link
Member

onPageChange, onObjectPageChange are part of the Book API but missing from the patch notes. Might be worth adding them into this PR seems as they're related:

https://steamcommunity.com/app/286160/eventcomments/1634166237645750353?ctp=4#c1637542851352346026

They're onPageChange() (no parameters) and onObjectPageChange(object)

- 'RPGFigurine': rpgfigurine.md
- 'TextTool': texttool.md
- 'Time': time.md
- 'Timer': timer.md
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we keep or not the Timer doc ?
@Benjamin-Dobell

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's reasonable to remove it. Having two options is confusing for newbies and they may not realise that Wait is a better solution.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's already gone from the nav bar on the main website

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should remove the .md also ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's deprecated but it still works, so i don't think we should

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. Maybe don't touch it in this PR then (it's unrelated to 12.3 anyway).

However, at the very least we should add something to state that it's deprecated; currently there's no indication that is the case.

Copy link
Author

@omniraptorr omniraptorr Apr 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing it from the nav bar (but not deleting the .md) just brings us in line with the main website. i think that's fine to leave in for this pr. we can add a warning that it's deprecated later

docs/book.md Outdated
Comment on lines 22 to 23
<a class="anchor" id="sethighlight"></a>setHighlight(x1, y1, x2, y2) | Set highlight box on current page. | [<span class="ret boo"></span>](types.md)
<a class="anchor" id="clearhighlight"></a>clearHighlight() | Clear highlight. | [<span class="ret boo"></span>](types.md)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add functions details for sethighlight and clearhighlight ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't know how they actually work. maybe benjamin can help

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I haven't looked into these specifically and am about to head off. I reckon don't let this hold up the pull request. We can always add more details later.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let my do some research about this method.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added some info about the setHighlight() function

@nabbydude
Copy link

did some testing and it seems like highlights only show when the PDF is popped out to screen, and don't work on game-load or after clearHighlight() until you create a highlight manually (by clicking and dragging). not sure if that fixes it for all players or just the highlining player.

@Zehir
Copy link

Zehir commented Apr 28, 2020

did some testing and it seems like highlights only show when the PDF is popped out to screen, and don't work on game-load or after clearHighlight() until you create a highlight manually (by clicking and dragging). not sure if that fixes it for all players or just the highlining player.

The Highlight is not saved.
The values are 2 points, (named 1 and 2). The first is bottom left, the second if top right. The value is between 0 and 1.

-- The bottom left corner will be highlighted (0,0 = bottom left corner and 0.5,0.5 = middle)
self.Book.setHighlight(0, 0, 0.5, 0.5)

@omniraptorr omniraptorr requested a review from Zehir April 29, 2020 04:19
@omniraptorr omniraptorr merged commit f832ac9 into master Apr 29, 2020
@omniraptorr omniraptorr deleted the patch_12.3 branch April 29, 2020 15:22
Benjamin-Dobell pushed a commit to Berserk-Games/Tabletop-Simulator-API that referenced this pull request May 16, 2020
adds in new lua features from patch 12.3. Since this patch introduces a couple new types of spawnable objects, we also move spawnable objects to their own subcategory in the scripting api nav menu.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Patch Notes v12.3

5 participants