Skip to content

Conversation

@Licini
Copy link
Contributor

@Licini Licini commented Jan 25, 2024

@tomvanmele Changes you requested:

  • Moved registration to get_scene_object_cls so it will always be called.
  • Renamed redraw to draw, and kept it as alias.
  • Added pluggables predraw and postdraw.
  • Also raise TypeError if item is not an Data, as alternative solution to allow non-Data item in SceneObject #1274 .


return drawn_objects

redraw = draw
Copy link
Contributor Author

Choose a reason for hiding this comment

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

But to be honest, I would even vote for not keep this alias, to distinct it from the redraw pluggable

Copy link
Member

Choose a reason for hiding this comment

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

Agree

@tomvanmele
Copy link
Member

do we even need the redraw pluggable? rhino's redraw can be triggered by the postdraw hook...



@pluggable(category="drawing-utils")
def predraw():
Copy link
Member

Choose a reason for hiding this comment

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

Docstrings to explain what these do?

Copy link
Member

Choose a reason for hiding this comment

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

@Licini and please add them to __init__ and to the api docs.



@pluggable(category="drawing-utils")
def postdraw():
Copy link
Member

Choose a reason for hiding this comment

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

If these are hooks, I would call them before_draw and after_draw

Copy link
Member

@gonzalocasas gonzalocasas left a comment

Choose a reason for hiding this comment

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

LGTM

@Licini
Copy link
Contributor Author

Licini commented Jan 26, 2024

@tomvanmele All adjusted, take a final look?

@Licini Licini merged commit 81e33c0 into main Jan 26, 2024
@Licini Licini deleted the draw branch January 26, 2024 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants