Page MenuHome

Remove deprecated ghosting code
ClosedPublic

Authored by Jacques Lucke (JacquesLucke) on Jan 28 2019, 3:50 PM.

Details

Summary

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?

Diff Detail

Repository
rB Blender

Event Timeline

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.

@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?

I don't mind removing it entirely if @Joshua Leung (aligorith) is fine with it. We always have git history.

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.

  • 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?

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.

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.

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.

@Brecht Van Lommel (brecht), you mean removing them instead of marking them as DNA_DEPRECATED?

Yes. DNA_DEPRECATED is only needed for backwards compatibility in versioning code, which we do not care about in this case.

  • remove versioning code and deprecated properties
  • better padding (same as before last commit)
This revision is now accepted and ready to land.Jan 31 2019, 2:23 PM
Jacques Lucke (JacquesLucke) retitled this revision from Remove ghosting settings from UI to Remove deprecated ghosting code.Jan 31 2019, 2:27 PM

@Joshua Leung (aligorith), I'll merge this tomorrow if you don't say anything that speaks against it.

Overall, I'm happy for this to go ahead.

source/blender/blenloader/intern/versioning_250.c
1255

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.

This revision was automatically updated to reflect the committed changes.