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.

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

Allow Indirect Field Accesses in Proto Templates #6614

Conversation

CoolSpy3
Copy link
Contributor

@CoolSpy3 CoolSpy3 commented Aug 4, 2024

Description
Allows use of the raw fields variable in proto templates.

Related Issues
This pull-request fixes issue #6566.

Tasks
Add the list of tasks of this PR.

  • Allow the use of the fields variable in proto templates.
  • Add tests for the new behavior
  • Update the documentation (By my reading of the current documentation [lua] [javascript], the new behavior is already described.) Document the new tag
  • Clarify proto regeneration behavior
  • Update the changelog

Documentation
Procedural PROTO Nodes (Lua)
Procedural PROTO Nodes (Javascript)
("Programming Facts" and "PROTO Regeneration" sections)

@CoolSpy3 CoolSpy3 marked this pull request as ready for review August 4, 2024 00:56
@CoolSpy3 CoolSpy3 requested a review from a team as a code owner August 4, 2024 00:56
@CoolSpy3 CoolSpy3 added the test suite Start the test suite label Aug 4, 2024
@CoolSpy3
Copy link
Contributor Author

CoolSpy3 commented Aug 4, 2024

The CI failure does appear to be reproducible on this branch and not master, so I'm marking this PR as a draft until I have time to investigate it.

@CoolSpy3 CoolSpy3 marked this pull request as draft August 4, 2024 02:19
@CoolSpy3
Copy link
Contributor Author

CoolSpy3 commented Aug 4, 2024

It looks like the Car proto is being regenerated as a result of these changes, causing the controller to restart. Thus, the test supervisor thinks there's three controllers running, but it only receives completion reports from two of them. I still have to find a good workaround. The only real solution I can think of is gating the behavior behind a tag. (i.e. # tags: fieldsObject/# tags: indirectFieldAccess [open to suggestions on the name]). However, if someone else has another idea, feel free to let me know.

Side note: In general, proto robots having their controllers restarted when they're regenerated seems a little unintuitive, however, any proposed behavior I can think of has some annoying edge cases. I think the best option is just to make a note of it in the proto docs. If others feel that we should update this behavior in some way, one of us can open a new issue with the proposed behavior.

@CoolSpy3 CoolSpy3 marked this pull request as ready for review August 4, 2024 22:40
@CoolSpy3
Copy link
Contributor Author

CoolSpy3 commented Aug 4, 2024

Note: It's hard to tell from the diff, but WbProtoModel:219 (Now 227) and WbProtoModel:281 (Now 292) are not identical to their pre-edit counterparts.

Copy link
Member

@omichel omichel left a comment

Choose a reason for hiding this comment

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

It looks good to me.

tests/protos/worlds/template_indirect_field_access_lua.wbt Outdated Show resolved Hide resolved
tests/protos/worlds/template_indirect_field_access.wbt Outdated Show resolved Hide resolved
Co-authored-by: Olivier Michel <[email protected]>
@CoolSpy3 CoolSpy3 requested a review from omichel August 12, 2024 19:05
Copy link
Member

@omichel omichel left a comment

Choose a reason for hiding this comment

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

Thank you.

@CoolSpy3 CoolSpy3 merged commit fe2879c into cyberbotics:master Aug 12, 2024
22 checks passed
@CoolSpy3 CoolSpy3 deleted the fix-proto-template-indirect-field-references branch September 7, 2024 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test suite Start the test suite
Development

Successfully merging this pull request may close these issues.

Can't reference fields from PROTO javascript functions if not referenced directly
2 participants