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

Refactored EditorHistory, removed unused code, added documentation. #44222

Conversation

EricEzaM
Copy link
Contributor

@EricEzaM EricEzaM commented Dec 9, 2020

Trying to document and refactor a bit of the editor so it is easier for people to work with.

Starting small, with the EditorHistory.

  • HISTORY_MAX enum was never used
  • EditorHistory::PropertyData struct was never used
  • add_object method variants with p_level_change were never used, and level property never changes. level property is essentially just a convenience for history[current].path.size() - 1 (i.e. last item in path)
  • add_object method variants were so rarely used, it made more sense to make the underlying _add_object public and just give it default parameters. This required only one external change, where add_object_inspector_only was used in EditorNode.
  • Spent a bit of time trying to figure out how it works. There should be no behavioural/functional changes.

@EricEzaM EricEzaM force-pushed the PR/editor-history-refactor-and-doco branch 2 times, most recently from b47a5f3 to db5c67a Compare December 9, 2020 14:30
@EricEzaM EricEzaM force-pushed the PR/editor-history-refactor-and-doco branch from db5c67a to 588f9f0 Compare December 9, 2020 14:40
@Calinou Calinou added this to the 4.0 milestone Dec 9, 2020
Comment on lines -61 to -67
if (j <= history[i].level) {
//before or equal level, complete fail
fail = true;
} else {
//after level, clip
history.write[i].path.resize(j);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already established that level is the last item in the path. Thus the first if will always be true.

@akien-mga
Copy link
Member

Superseded by #58395.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants