Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for sceneSubBoundingRect and other functions requiring params, fix failing test on 12.5 #119

Merged
merged 3 commits into from
Nov 17, 2023
Merged

add support for sceneSubBoundingRect and other functions requiring params, fix failing test on 12.5 #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e2f9062
add support for sceneSubBoundingRect and other functions requiring pa…
triwav Nov 17, 2023
d014771
remove debug code
triwav Nov 17, 2023
6c2ba34
Add param typing and documentation
triwav Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions client/src/OnDeviceComponent.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,24 +742,24 @@ describe('OnDeviceComponent', function () {
backExitsScene: 'boolean',
backgroundColor: 'color',
backgroundUri: 'uri',
change: 'string',
childRenderOrder: 'string',
change: 'std::type_index',
childRenderOrder: 'std::type_index',
clippingRect: 'rect2d',
currentDesignResolution: 'std::shared_ptr<std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::any, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::any> > > >',
dialog: 'std::shared_ptr<Roku::SceneGraph::DialogBase>',
currentDesignResolution: 'std::type_index',
dialog: 'std::type_index',
enableRenderTracking: 'boolean',
focusable: 'boolean',
focusedChild: 'node',
focusedChild: 'std::type_index',
id: 'string',
inheritParentOpacity: 'boolean',
inheritParentTransform: 'boolean',
limitBackgroundToUIResolution: 'boolean',
muteAudioGuide: 'boolean',
opacity: 'float',
pagesContainer: 'node',
palette: 'std::shared_ptr<Roku::SceneGraph::RSGPalette>',
palette: 'std::type_index',
renderPass: 'integer',
renderTracking: 'string',
renderTracking: 'std::type_index',
rotation: 'float',
scale: 'vector2d',
scaleRotateCenter: 'vector2d',
Expand Down Expand Up @@ -834,6 +834,21 @@ describe('OnDeviceComponent', function () {
expect(found).to.false;
});
});

describe('sceneSubBoundingRect()', () => {
it('should work on node item', async () => {
const { value } = await odc.getValue({ keyPath: '#rowListWithCustomTitleComponent.sceneSubBoundingRect(item1_1)' });
expect(value.height).to.equal(150);
expect(value.width).to.equal(300);
expect(value.x).to.equal(480);
expect(value.y).to.equal(936);
});

it('should gracefully fallback if called on nonsupported type', async () => {
const { found } = await odc.getValue({ base: 'global', keyPath: 'intValue.sceneSubBoundingRect(item0_1)()' });
expect(found).to.false;
});
});
});
});

Expand Down
47 changes: 32 additions & 15 deletions device/components/RTA_helpers.brs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,75 +321,91 @@ end function
' * @description Helper for RTA_getValueAtKeyPath to break out
' * @param {Object} key - Key of the function the user is asking us to call
' * @param {Dynamic} level - The variable we are calling the function call on
' * @param {Integer} openingParenthesisPosition - Where the opening paranthesis are located in keyPathPart
' */
function RTA_callBrightscriptInterfaceFunction(functionName as string, callOn as Dynamic) as Dynamic
if functionName = "getParent()" then
function RTA_callBrightscriptInterfaceFunction(keyPathPart as string, callOn as Dynamic, openingParenthesisPosition as Integer) as Dynamic
functionName = left(keyPathPart, openingParenthesisPosition)
closingParenthesisPosition = keyPathPart.len()
if mid(keyPathPart, closingParenthesisPosition) <> ")" then
RTA_logWarn("Could not find closing parenthesis" + keyPathPart)
return Invalid
end if
numCharacters = closingParenthesisPosition - (openingParenthesisPosition + 2)
functionParams = mid(keyPathPart, openingParenthesisPosition + 2, numCharacters).tokenize(",")

if functionName = "getParent" then
if RTA_isNode(callOn) then
return callOn.getParent()
else
RTA_logWarn("tried to call getParent() on non node of type " + type(callOn))
end if
else if functionName = "count()" then
else if functionName = "count" then
Copy link
Contributor

Choose a reason for hiding this comment

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

I like seeing all the paren optimizations here.

if RTA_isArray(callOn) OR RTA_isKeyedValueType(callOn) then
return callOn.count()
else
RTA_logWarn("tried to call count() on non AA or array of type " + type(callOn))
end if
else if functionName = "keys()" then
else if functionName = "keys" then
if RTA_isNode(callOn) then
return callOn.keys().toArray() ' keys() returns an roList when called on a node. We have to convert to array as all of our array checks are specifically looking for an array
else if RTA_isAA(callOn) then
return callOn.keys()
else
RTA_logWarn("tried to call keys() on non keyed value of type " + type(callOn))
end if
else if functionName = "len()" then
else if functionName = "len" then
if RTA_isString(callOn) then
return callOn.len()
else
RTA_logWarn("tried to call len() on non string of type " + type(callOn))
end if
else if functionName = "getChildCount()" then
else if functionName = "getChildCount" then
if RTA_isNode(callOn) then
return callOn.getChildCount()
else
RTA_logWarn("tried to call getChildCount() on non node of type " + type(callOn))
end if
else if functionName = "threadinfo()" then
else if functionName = "threadinfo" then
if RTA_isNode(callOn) then
return callOn.threadinfo()
else
RTA_logWarn("tried to call threadinfo() on non node of type " + type(callOn))
end if
else if functionName = "getFieldTypes()" then
else if functionName = "getFieldTypes" then
if RTA_isNode(callOn) then
return callOn.getFieldTypes()
else
RTA_logWarn("tried to call getFieldTypes() on non node of type " + type(callOn))
end if
else if functionName = "subtype()" then
else if functionName = "subtype" then
if RTA_isNode(callOn) then
return callOn.subtype()
else
RTA_logWarn("tried to call subtype() on non node of type " + type(callOn))
end if
else if functionName = "boundingRect()" then
else if functionName = "boundingRect" then
if RTA_isNode(callOn) then
return callOn.boundingRect()
else
RTA_logWarn("tried to call boundingRect() on non node of type " + type(callOn))
end if
else if functionName = "localBoundingRect()" then
else if functionName = "localBoundingRect" then
if RTA_isNode(callOn) then
return callOn.localBoundingRect()
else
RTA_logWarn("tried to call localBoundingRect() on non node of type " + type(callOn))
end if
else if functionName = "sceneBoundingRect()" then
else if functionName = "sceneBoundingRect" then
if RTA_isNode(callOn) then
return callOn.sceneBoundingRect()
else
RTA_logWarn("tried to call sceneBoundingRect() on non node of type " + type(callOn))
RTA_logWarn("tried to call sceneBoundingRect on non node of type " + type(callOn))
end if
else if functionName = "sceneSubBoundingRect" then
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way we could add better support for any function, rather than needing to hardcode specific functions? This list might become a bit unmanageable over time. Just curious.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I believe that was one of the things Roku was asking us to rank. Currently there is no way to call a function from a param. I wouldn't be surprised if we never get it though as they use the fact that you have to use an exact string for a function call in their static analysis checks.

if RTA_isNode(callOn) then
return callOn.sceneSubBoundingRect(functionParams[0])
else
RTA_logWarn("tried to call sceneBoundingRect on non node of type " + type(callOn))
end if
else
RTA_logWarn("tried to call unknown function" + functionName)
Expand All @@ -416,8 +432,9 @@ function RTA_getValueAtKeyPath(base as Object, keyPath as String, fallback = Inv
while NOT keys.isEmpty()
key = keys.shift()
' Check for any Brightscript interface function calls
if key.Instr("()") > 0 then
level = RTA_callBrightscriptInterfaceFunction(key, level)
openingParenthesisPosition = key.Instr("(")
if openingParenthesisPosition > 0 then
level = RTA_callBrightscriptInterfaceFunction(key, level, openingParenthesisPosition)
else if RTA_isKeyedValueType(level) then
nextLevel = level[key]
if nextLevel = Invalid AND RTA_isNode(level) then
Expand Down