Nate Rupsis (nrupsis) retitled this revision from default N-panel open for Graph and NLA editors to default N-panel open for animation editors.May 26 2022, 4:39 PM

Nate Rupsis (nrupsis) edited the summary of this revision. (Show Details)

Nate Rupsis (nrupsis) added reviewers: Sybren A. Stüvel (sybren), Brad Clark (RiggingDojo), Kevin C. Burke (blastframe).

Nate Rupsis (nrupsis) added a project: Animation & Rigging.

Brad Clark (RiggingDojo) awarded a token.May 26 2022, 5:08 PM

LGTM! Just a few small things in the inline notes.

I want to propose that you look at the "weirdness" you described in T97980#1363230, to see if you can fix it. Even though that fix belongs to a different patch, I think it's a good idea to land that patch and this one simultaneously, in consecutive commits. That way we expose as few people as possible to the "weirdness".

source/blender/blenloader/intern/versioning_defaults.c
138	end with period
source/blender/makesrna/intern/rna_space.c
2298 ↗	(On Diff #51871)	Please add a comment that explains why the timeline is a special case here.

This revision is now accepted and ready to land.May 27 2022, 12:27 PM

Accepted the patch too soon; if you had commit rights I'd say "update the comments before you commit", but now I think it's more confusing than anything to have some request for changes and an "accepted" state at the same time.

This revision now requires changes to proceed.May 27 2022, 12:29 PM

comment cleanup

Harbormaster completed remote builds in B22423: Diff 52238.Jun 3 2022, 6:10 PM

Sybren A. Stüvel (sybren) accepted this revision.Jun 23 2022, 1:10 PM

This revision is now accepted and ready to land.Jun 23 2022, 1:10 PM

Andres Stephens (Draise) awarded a token.Jun 25 2022, 6:03 PM

I did some more testing, and there are still some issues with the patch (apologies for the accept/reject cycle that's going on here).

In current master, this is the behaviour:

Dope Sheet has panel hidden.
Press N, panel is shown.
Change mode a few times, to Action, Grease Pencil, etc.
See that the panel remains visible.
Press N, panel is hidden.
Change mode a few times, to Action, Grease Pencil, etc.
See that the panel remains hidden.

With this patch, this is broken. Every time the mode changes, the panel is shown again. The approach of opening the panel in rna_SpaceDopeSheetEditor_mode_update is not the right one; we should be using a place that is run as a one-off thing. The versioning code seems a good candidate for this.

This diff, applied on top of the current patch, seems to do the trick:

P3043 Open N-panel by default in animation editors

1	diff --git a/source/blender/blenloader/intern/versioning_300.c b/source/blender/blenloader/intern/versioning_300.c
2	index 68df560a389..7a29afb4048 100644
3	--- a/source/blender/blenloader/intern/versioning_300.c
4	+++ b/source/blender/blenloader/intern/versioning_300.c
5	@@ -3243,5 +3243,33 @@ void blo_do_versions_300(FileData fd, Library UNUSED(lib), Main *bmain)
6	*/
7	{
8	/* Keep this block, even when empty. */
9	+
10	+ {
11	+ /* In the Dope Sheet, for every mode other than Timeline, open the Properties panel. */
12	+ LISTBASE_FOREACH (bScreen *, screen, &bmain->screens) {
13	+ LISTBASE_FOREACH (ScrArea *, area, &screen->areabase) {
14	+ LISTBASE_FOREACH (SpaceLink *, sl, &area->spacedata) {
15	+ if (sl->spacetype != SPACE_ACTION) {
16	+ continue;
17	+ }
18	+
19	+ /* Skip the timeline, it shouldn't get its Properties panel opened. */
20	+ SpaceAction saction = (SpaceAction )sl;
21	+ if (saction->mode == SACTCONT_TIMELINE) {
22	+ continue;
23	+ }
24	+
25	+ const bool is_first_space = sl == area->spacedata.first;
26	+ ListBase *regionbase = is_first_space ? &area->regionbase : &sl->regionbase;
27	+ ARegion *region = BKE_region_find_in_listbase_by_type(regionbase, RGN_TYPE_UI);
28	+ if (region == NULL) {
29	+ continue;
30	+ }
31	+
32	+ region->flag &= ~RGN_FLAG_HIDDEN;
33	+ }
34	+ }
35	+ }
36	+ }
37	}
38	}
39	diff --git a/source/blender/makesrna/intern/rna_space.c b/source/blender/makesrna/intern/rna_space.c
40	index 20bf13145fd..745c7137cb2 100644
41	--- a/source/blender/makesrna/intern/rna_space.c
42	+++ b/source/blender/makesrna/intern/rna_space.c
43	@@ -2293,17 +2293,6 @@ static void rna_SpaceDopeSheetEditor_mode_update(bContext C, PointerRNA ptr)
44	}
45	ED_region_visibility_change_update(C, area, channels_region);
46	}
47	- /* For every mode other than the timeline, open the properties panel by default. */
48	- ARegion *ui_region = BKE_area_find_region_type(area, RGN_TYPE_UI);
49	- if (ui_region) {
50	- if (saction->mode == SACTCONT_TIMELINE) {
51	- ui_region->flag \|= RGN_FLAG_HIDDEN;
52	- }
53	- else {
54	- ui_region->flag &= ~RGN_FLAG_HIDDEN;
55	- }
56	- ED_region_visibility_change_update(C, area, ui_region);
57	- }
58	}
59
60	/* recalculate extents of channel list */

To make it work as a one-time thing, the block in versioning_300.c would have to be moved into a subversion check (if (!MAIN_VERSION_ATLEAST(bmain, x, y)) { ... }) and the subversion would have to be bumped. Otherwise it'll reset people's choices every time a file is opened. As this is sensitive to collisions with other commits, I'll do that when landing.

This revision now requires changes to proceed.Jun 30 2022, 11:19 AM

applying Sybrens changes, as disscussed in Animation module meeting 2022-07-07

Harbormaster completed remote builds in B22913: Diff 53439.Jul 8 2022, 4:12 PM

fixing partially applied patch

Harbormaster completed remote builds in B22914: Diff 53440.Jul 8 2022, 4:14 PM

This revision was not accepted when it landed; it landed in state Needs Review.Jul 26 2022, 11:45 AM

Closed by commit rBb08c5381ac7f: default N-panel open for animation editors (authored by Nate Rupsis (nrupsis), committed by Sybren A. Stüvel (sybren)). · Explain Why

This revision was automatically updated to reflect the committed changes.

Sybren A. Stüvel (sybren) added a commit: rBb08c5381ac7f: default N-panel open for animation editors.

default N-panel open for animation editors
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 54057

source/blender/blenloader/intern/versioning_300.c

source/blender/blenloader/intern/versioning_defaults.c

source/blender/editors/space_action/space_action.c

source/blender/editors/space_graph/space_graph.c

source/blender/editors/space_nla/space_nla.c

default N-panel open for animation editorsClosedPublicActions