Skip to content

Conversation

vasarhelyi
Copy link
Contributor

@vasarhelyi vasarhelyi commented Sep 2, 2025

This PR changes a fundamental concept in how colors are stored for Skybrush drone shows: we do not have a detached material for each drone any more, but have a common template material only, and use a different shader node tree that inputs object.color into the node tree. This should speed up color and light effects handling and export substantially! The PR is not ready, TODOs are summarized below:

  • users should be notified to change Solid Viewport Shading mode's "Object Color" to "Object" by default, to see the colors properly. In script: bpy.context.space_data.shading.color_type = 'OBJECT'
  • a migration operator needs to be created to convert animation of old shows to new workflow and users should be asked automatically whether they wish to convert a show when they open an old show
  • now I create an action for each drone for its color animation, but I have not checked whether it gets into conflict with other actions we might have created previously for other reasons. @ntamas can you comment on this?
  • once all features above are ready, old code that is not used any more related to materials can be removed
  • documentation needs to be updated
  • general code review and testing

use a single common material for all drones and animate drone.color instead, with injecting that into the shader node trees of objects

BREAKING CHANGE: old files are not compatible with this change yet
…terial per drone to new style with shared material
…of the 3D viewport on migration to new color-handling
@vasarhelyi
Copy link
Contributor Author

vasarhelyi commented Sep 3, 2025

users should be notified to change Solid Viewport Shading mode's "Object Color" to "Object" by default, to see the colors properly. In script: bpy.context.space_data.shading.color_type = 'OBJECT'

I created a function for that migration, see set_all_shading_color_types_to_object(). @ntamas perhaps we should put an automatic call for this function in the new version upon loading the plugin. Where exactly?

@vasarhelyi
Copy link
Contributor Author

vasarhelyi commented Sep 3, 2025

a migration operator needs to be created to convert animation of old shows to new workflow and users should be asked automatically whether they wish to convert a show when they open an old show

The operation is ready, called UseSharedMaterialForAllDronesMigrationOperator currently. @ntamas please review code and naming and code structure. I expect that we might have other migration operators as well in the future, it would be nice to handle these properly in a general way. Where could we put a call to this on the UI or how should we inform the users automatically to call this to upgrade their old files? (currently available for testing through F3 only)

@vasarhelyi vasarhelyi requested a review from ntamas September 3, 2025 09:21
@ntamas
Copy link
Member

ntamas commented Sep 4, 2025

perhaps we should put an automatic call for this function in the new version upon loading the plugin. Where exactly?

Looks like you've found it already :) (initialization.py)

Where could we put a call to this on the UI or how should we inform the users automatically to call this to upgrade their old files?

I think that it is probably time to introduce a version number in DroneShowAddonProperties, starting from 1, and then the migration operators only need to specify a version number threshold such that they get executed automatically when the version number in DroneShowAddonProperties is missing or less than the version number required by the migration operator. (Of course they must also update the version number in the file after they were executed successfully).

@vasarhelyi
Copy link
Contributor Author

I added a version property and automated migrations based on the version numbers. Might be over complicated, but it works now. Migration always starts from version 1 and should call all migration operators, which automatically decide to run and bump or just bump or not touch the version etc. Tested with old files, new files, empty files, seem to be working now.

update_pyro_particles_of_object(drone)


def config_logging(*args):
Copy link
Member

Choose a reason for hiding this comment

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

Do not mess around with global settings like logging configuration. If you need it for development purposes, that's okay, but do not invoke it unconditionally for end users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, thx

def set_all_shading_color_types_to_object() -> None:
"""Sets the object color source of the solid and wireframe
shading types of the 3D Viewport to use object color."""
shading = getattr(bpy.context.space_data, "shading", None)
Copy link
Member

Choose a reason for hiding this comment

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

bpy.context.space_data might be a completely different object depending on how the workspace of the user is set up. We need to find the 3D viewport with find_one_3d_view() or find_all_3d_views() and then work on those.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@vasarhelyi
Copy link
Contributor Author

@ntamas all your comments answered so far, and also added warnings properly. Please recheck.

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.

2 participants