Page MenuHome
Differential D11109

"File > External Data" menu improvements and cleanup
ClosedPublic
Actions

Authored by Dalai Felinto (dfelinto) on Apr 28 2021, 4:04 PM.

Details

Reviewers
Sybren A. Stüvel (sybren)
Commits
rBee51e7335552: "File > External Data" menu improvements and cleanup
Summary

All changes:

  • Include file.pack_libraries and file.unpack_libraries to the menu [1].
  • Rename "Pack Blender Libraries" → "Pack Linked Libraries" [2].
  • Rename "Unpack Blender Libraries" → "Unpack Linked Libraries" [2].
  • Rename "Pack All Into .blend" → "Pack Resources" [3]
  • Rename "Unpack All Into Files" → "Unpack Resources" [3]
  • Rename "☑ Automatically Pack Into .blend" → "☑ Automatically Pack Resources" [3]
  • Rename "Make All Paths Relative" → "Make Paths Relative" [4]
  • Rename "Make All Paths Absolute" → "Make Paths Absolute" [4]
  • Add separators accordingly

[1] - This was never exposed since its original commit rB16411da41e40. Now that operator not listed in menus don't
show up in the search, this became even more hidden.

[2] - The original name (Pack Blender Library) was not clear enough. Pose Libraries and Asset Libraries are also technically Blender libraries.

[3] - The term All was misleading since it didn't include the Linked Libraries.

[4] - No need to use "All". It is not used in the Report/Find Missing Files either.

This commit put this in the File > External Data menu.

Diff Detail

Repository
rB Blender
Branch
pack-external (branched from master)
Build Status
Buildable 14372
Build 14372: arc lint + arc unit

Event Timeline

Dalai Felinto (dfelinto) requested review of this revision.Apr 28 2021, 4:04 PM
Dalai Felinto (dfelinto) created this revision.
Harbormaster completed remote builds in B14278: Diff 36606.Apr 28 2021, 4:04 PM
Dalai Felinto (dfelinto) added a reviewer: Sybren A. Stüvel (sybren).Apr 28 2021, 4:05 PM

Dalai Felinto (dfelinto) added a project: User Interface.Apr 28 2021, 4:23 PM
Julian Eisel (Severin) added a subscriber: Julian Eisel (Severin).Apr 28 2021, 5:20 PM

+1 in general.

  • Do we know how well this is working? Given that it was kinda hidden, I wouldn't be surprised if something was broken.
  • Using "Blender Libraries" is a bit odd & redundant. Just "Pack/Unpack Libraries"? Although to avoid confusion with asset libraries I'd prefer "Data-Block Libraries" even.
  • I find the distinction to other pack/unpack operators confusing. E.g. "Pack All Into .blend" packs image files, sound files, etc. but not library files. However now we add an operator that says you can also pack library files, it's just not included in the "Pack All" operator. Maybe we can name it differently? Something like "Merge/Unmerge Data-Block Libraries"? Not sure.
Julian Eisel (Severin) added a comment.Apr 28 2021, 5:23 PM

We could also rename "Pack All Into .blend" to "Pack Non-Blender Files Into .blend" (and unpack accordingly). Not as catchy but more descriptive.

Sybren A. Stüvel (sybren) requested changes to this revision.Apr 28 2021, 5:29 PM

+1 on the issue that with this patch, there is "all" and then there is "yeah but 'all' didn't actually include everything, this operator does the other stuff". I think a relabeling is in order.

This has already been added to the 2.93 release notes, which means this is going to applied to 2.93? That would have been nice to mention in the patch description.

Even though this is not a new feature (Ton implemented it in 2012), it will be new to a lot of people. Does it have documentation in the manual?

This revision now requires changes to proceed.Apr 28 2021, 5:29 PM
Dalai Felinto (dfelinto) added a comment.Apr 28 2021, 5:45 PM

@Julian Eisel (Severin) this feature is still used often to share files to the (Blender) cloud. This is the feedback I got today from Andy.

I think the proper naming could be:

  • Pack data-blocks into .blend
  • Pack [Blender] libraries.

Now, if we are even considering to rename something as fundamental as the existing "Pack All Into .blend" I would say this patch should be for 3.0 then. I commented out the mention in the wiki meanwhile.

Harley Acheson (harley) added a subscriber: Harley Acheson (harley).Apr 28 2021, 6:01 PM

Keep in mind that I doubt that I have used these operators at all. So you can only take the following comments as “what an uninformed and confused newbie might say”. But I find lots of weird things about all these options. All of the “All” are confusing. Why can I “Make All Paths Relative” if I can’t make just some of them so? There is an assumption of knowing what packing and unpacking means. So my unformed, and probably wrong, suggestions:

☐ Automatically Pack External Resources
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Pack External Resources into this Blend
Extract Packed Resources into Files
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Pack Dependent Blend files into this one
Extract Packed Blends into Files
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Make Resource File Paths Relative
Make Resource File Paths Absolute
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Report Missing External Resource Files
Find Missing External Resource Files

Sybren A. Stüvel (sybren) added a comment.May 3 2021, 4:08 PM
In D11109#283834, @Dalai Felinto (dfelinto) wrote:

This is the feedback I got today from Andy.

I think the proper naming could be:

  • Pack data-blocks into .blend
  • Pack [Blender] libraries.

What would "Pack data-blocks into .blend" do? Because the "Pack All" operator would pack non-datablock data (like the actual PNG file, contrary to the datablock referring to it).

Now, if we are even considering to rename something as fundamental as the existing "Pack All Into .blend" I would say this patch should be for 3.0 then.

I agree.

In D11109#283849, @Harley Acheson (harley) wrote:

I find lots of weird things about all these options. All of the “All” are confusing.

I agree.

So my unformed, and probably wrong, suggestions:
☐ Automatically Pack External Resources
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Pack External Resources into this Blend
Extract Packed Resources into Files

There is always the issue that "resources" is equally ill-defined. In this case it means "external files but not blend files"; you could just as well see blend files as resources of your project. I think your suggestion for "external resources" is already an improvement over the current all-but-not-really-all. We could explain the "but not blend files" part in the tooltip, maybe that is enough.

Pack Dependent Blend files into this one
Extract Packed Blends into Files

The concept of "libraries" is used in other places in the UI as well, so I wouldn't start naming them differently for these operators.

Make Resource File Paths Relative
Make Resource File Paths Absolute

I think (but I may be wrong) that these operators actually work on all paths, so including libraries. So in this case using the word "All" would be correct, when we define "All" to be "Resources + Libraries".

Philipp Oeser (lichtwerk) added subscribers: Bastien Montagne (mont29), Brecht Van Lommel (brecht), Philipp Oeser (lichtwerk).May 3 2021, 4:25 PM

We had this already, see D9279: Add 'Pack/Unpack Blender Libraries' to the File Menu.
Long story short: @Bastien Montagne (mont29) wanted some more extensive testing

In D9279#232110, @Bastien Montagne (mont29) wrote:

I currently have no idea how well this feature is supported ? Would like to have it tested a bit first, including checking how it deals with complex cases (hierarchy of multiple libraries, how actions like reload and relocate are behaving with packed libraries, how overrides are behaving with packed libraries, etc.)

My guts tell me that exposing/actually using this could be a nice can of worms that I would rather not open right now - but I might be wrong of course.

Otoh:

In D9279#232158, @Brecht Van Lommel (brecht) wrote:

The Agent 327 demo files on blender.org and similar files on the Blender Cloud are packed with this, so it has had some testing in complex files.

Dalai Felinto (dfelinto) added a comment.May 3 2021, 5:18 PM

Oh I didn't see your patch Philipp. Anyways, the feature got enough testing, as mentioned in your patch. All it is pending is the proper naming indeed (and someone with energy to bring this discussion all the way to the end).

Dalai Felinto (dfelinto) updated this revision to Diff 36734.May 3 2021, 5:52 PM
  • Trying to clarify the differences between pack modes
Harbormaster completed remote builds in B14347: Diff 36734.May 3 2021, 5:52 PM
Dalai Felinto (dfelinto) edited the summary of this revision. (Show Details)May 3 2021, 5:56 PM
Dalai Felinto (dfelinto) updated this revision to Diff 36736.May 3 2021, 7:07 PM
  • Another pass: Pack Resources / Unpack Resources
Harbormaster completed remote builds in B14348: Diff 36736.May 3 2021, 7:07 PM
Dalai Felinto (dfelinto) edited the summary of this revision. (Show Details)May 3 2021, 7:12 PM
Harley Acheson (harley) added a comment.May 3 2021, 8:05 PM

Just to add a further comment to my earlier naïve ones...

By adding "Into this blend" and "into files" to the first Pack and Unpack items, it not only describes the process but informs on the meanings those words "Pack" and "Unpack" in this context. So you don't have to add those to later ones, just using "Pack" and "Unpack" are enough after the initial usages.

Without this clarification though, users could assume that "packing" was:

  • compression
  • defragmenting
  • removal of unneeded things
  • protection or some kind
  • to fill completely

It is quite a generic word with a very specific usage here that isn't clear without further explanation.

The tooltip descriptions don't really help much: "Unpack all files packed into this .blend to external ones" requires that the user understand what "unpack" means as well as "pack" and "to external ones" is quite vague.

Sybren A. Stüvel (sybren) added a comment.EditedMay 4 2021, 12:04 PM

It is quite a generic word with a very specific usage here that isn't clear without further explanation.

👍 Thanks for keeping an eye on this @Harley Acheson (harley), IMO this kind of feedback is very valuable. It's hard to find people who know how to report here and still can look at things with naïve eyes.

For me personally I think it's enough to include the "into this blend file" explanation in the tooltip. I agree with Harley that this is important to include. Update: it's already included there, so I'm happy :)

Sybren A. Stüvel (sybren) accepted this revision.May 4 2021, 12:06 PM
This revision is now accepted and ready to land.May 4 2021, 12:06 PM
Dalai Felinto (dfelinto) updated this revision to Diff 36760.May 4 2021, 3:17 PM

Final pass: Linked Libraries and Sanitize Paths labels as well

After further discussion Blender Library is better referred to as
Linked Library. Also removed the "All" from the path operators:

  • Make All Paths Relative > Make Paths Relative
  • Make All Paths Absolute > Make Paths Absolute
Harbormaster completed remote builds in B14367: Diff 36760.May 4 2021, 3:17 PM
Dalai Felinto (dfelinto) retitled this revision from Expose pack/unpack Blender libraries operators to the menu to "File > External Data" menu improvements and cleanup.May 4 2021, 3:27 PM
Dalai Felinto (dfelinto) edited the summary of this revision. (Show Details)
Julian Eisel (Severin) requested changes to this revision.May 4 2021, 3:39 PM

More back and forth :) This time on the descriptions.

The other descriptions in that menu aren't great either, but they are already exposed. So improving them is a separate thing IMO.

I quite like the proposed naming.

source/blender/editors/space_info/info_ops.c
75–77

This description isn't very useful, it's just a longer version of the operator name + the info that it's the current .blend it's packed into.
The description should actually explain what the feature does, i.e. take all data-blocks linked from other .blend files and store them in the current .blend file. Library references are preserved so the linked data-blocks can be unpacked again.

106

Same here.

This revision now requires changes to proceed.May 4 2021, 3:39 PM
Philipp Oeser (lichtwerk) mentioned this in D9279: Add 'Pack/Unpack Blender Libraries' to the File Menu.May 4 2021, 4:03 PM
Dalai Felinto (dfelinto) updated this revision to Diff 36769.May 4 2021, 4:07 PM

Improve Linked Library Tooltip

Harbormaster completed remote builds in B14372: Diff 36769.May 4 2021, 4:07 PM
Dalai Felinto (dfelinto) updated this revision to Diff 36774.May 4 2021, 4:12 PM

Typo

Harbormaster completed remote builds in B14376: Diff 36774.May 4 2021, 4:12 PM
Dalai Felinto (dfelinto) removed a reviewer: Julian Eisel (Severin).May 4 2021, 4:20 PM

I talked to Julien over chat and he was fine with the updated tooltip. (latest commit). I'm removing him from the reviewers to prevent this from being committed with a pending approval.

This revision is now accepted and ready to land.May 4 2021, 4:20 PM
Closed by commit rBee51e7335552: "File > External Data" menu improvements and cleanup (authored by Dalai Felinto (dfelinto)). · Explain WhyMay 4 2021, 4:21 PM
This revision was automatically updated to reflect the committed changes.
Dalai Felinto (dfelinto) added a commit: rBee51e7335552: "File > External Data" menu improvements and cleanup.
Dalai Felinto (dfelinto) added a comment.May 4 2021, 4:22 PM

For the records, user manual updated on rBM8002 and rBM8003.

Philipp Oeser (lichtwerk) mentioned this in T93282: Appending packed libraries is treated as if they weren't packed.Nov 22 2021, 12:19 PM

Revision Contents
Changeset List

PathSize
release/
scripts/
startup/
bl_ui/
10 lines
source/
blender/
editors/
space_info/
24 lines

Diff 36769

View Options

release/scripts/startup/bl_ui/space_topbar.py

Loading...
View Options

source/blender/editors/space_info/info_ops.c

Loading...