As discussed in T60481.
There was quite a bit of panel-code that wasn't used at all.
Question: Should the related RNA stuff be removed as well?
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- remove-ghosting (branched from master)
- Build Status
Buildable 2785 Build 2785: arc lint + arc unit
Event Timeline
Comment Actions
Question: Should the related RNA stuff be removed as well?
Yes, or at least put an #if 0 block around it with a comment explaining why it is default. Better to not have broken things in the public API.
Comment Actions
@Brecht Van Lommel (brecht), in that case I'd prefer to remove it completely, even from other parts of the C code and mark it as deprecated. Is there any reason to keep this code?
Comment Actions
I don't mind removing it entirely if @Joshua Leung (aligorith) is fine with it. We always have git history.
Comment Actions
I'm leaning towards completely removing it (i.e. the armature specific ghosting RNA/Py/etc.)
IIRC, we don't have any of the actual armature drawing code for ghosting hanging around anymore, so we might as well just get rid of the properties too.
Comment Actions
- remove ghosting code
Tell me when I removed too much, feels like I removed everything twice and I don't know why.
All variables I marked deprecated are only used in versioning code.
I'm not sure if the enums for e.g. armature->ghosttype should be removed as well.
They are often partially used by the versioning code.
I guess old versioning code should never be updated, right?
Or should the code be removed from there as well?
Comment Actions
Hmm... the bAnimViz ones can stay for now. If we restore such functionality, those will be the settings used. The ones on bArmature though are fair game.
The version patching stuff (especially converting from bArmature to bAnimViz can be skipped/removed.
Comment Actions
Is it possible to un-deprecate properties later on? Keeping code for a feature that might be implemented in the future resulted in this amount of unused code in the first place.
Personally, I think that the code can be brought back later when it becomes necessary? This is not very complicated code anyway.
Comment Actions
You can remove the struct members and old versioning code. There is no reason to preserve backwards compatibility for this.
I also think it's better to just remove the code and restore it if/when the feature gets implemented again.
Comment Actions
@Brecht Van Lommel (brecht), you mean removing them instead of marking them as DNA_DEPRECATED?
Comment Actions
Yes. DNA_DEPRECATED is only needed for backwards compatibility in versioning code, which we do not care about in this case.
Comment Actions
@Joshua Leung (aligorith), I'll merge this tomorrow if you don't say anything that speaks against it.
Comment Actions
Overall, I'm happy for this to go ahead.
| source/blender/blenloader/intern/versioning_250.c | ||
|---|---|---|
| 1263 ↗ | (On Diff #13452) | Hang on, why are we removing the path settings too? Granted, these have technically been deprecated since 2.5-ish anyway, and they're not really critical either. However, even though we're breaking a lot of backwards compatability with 2.8 anyway, I think it's still nice to try to maintain the ability to try to read old files as long as we can. The real question is - for how long should we keep trying? What's the current version of our general policy here now? It's probably fine to just let this go though, but I thought I'd just make a quick note about this here. |