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

Improve Bake operator activation #177

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

keianhzo
Copy link
Contributor

@keianhzo keianhzo commented Dec 8, 2022

Bake operators are not currently properly enabled/disabled based on the target objects state.

STRs:

  • Add a linked reflection probe to the scene
  • Go to the scene options
    Actual Result: The bake buttons are enabled
    Expected Result: The bake buttons shouldn't be enabled as the baking is not possible

This PR splits the current Bake operator in three different operators that have a different poll implementations.

This PRs splits the BakeOperator implementation in a base implementation and a concrete implementation for each bake type so we can poll properly. Although functional I'm not really happy with this implementation, I think there should be an easier way of making poll conditional but I couldn't find a better way based on my attempts below but I'm more than happy to know what should be (if any) the best way of doing this.

Other approaches considered:
The first intent was to store the bake mode in the context via context_pointer_set. This require a pointer type so storing a string/enum/int is not allowed:

            row = col.row()
            row.context_pointer_set('bake_op', 'ALL')
            bake_msg = "Baking..." if probe_baking and bake_mode == 'ALL' else "Bake All"
            bake_op = row.operator(
                "render.hubs_render_reflection_probe",
                text=bake_msg
            )
            bake_op.bake_mode = 'ALL'

Ideally we could store the operator properties reference but that would require the context_pointer_set to be called after the operator is created but in that case the context reference has already being bound and the context won't reflect the later context update:

            row = col.row()
            bake_msg = "Baking..." if probe_baking and bake_mode == 'ALL' else "Bake All"
            bake_op = row.operator(
                "render.hubs_render_reflection_probe",
                text=bake_msg
            )
            row.context_pointer_set('bake_op', bake_op)
            bake_op.bake_mode = 'ALL'

Another alternative would have been to add a property to the UILayout type to store the bake mode but doesn't seem reasonable to pollute the whole type with a narrow scoped property.:

            row = col.row()
            row.bake_mode = 'ALL'
            row.context_pointer_set('bake_data', row)
            bake_msg = "Baking..." if probe_baking and bake_mode == 'ALL' else "Bake All"
            bake_op = row.operator(
                "render.hubs_render_reflection_probe",
                text=bake_msg
            )

@keianhzo keianhzo changed the title Fix Bake operator activation Improve Bake operator activation Dec 8, 2022
@keianhzo keianhzo added Backlog and removed Backlog labels Mar 20, 2023
@Exairnous Exairnous self-requested a review April 27, 2023 07:09
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.

1 participant