deanljohnson
commented
6 years ago It shouldn’t have been - my mistake. I’m not in a place where I can fix it right now, but will update the PR later.
On Wednesday, January 17, 2018, Adam notifications@github.com wrote:
@ilexp commented on this pull request.
Looks very good, but wondering about one modified line, see below.
Good call for avoiding that potential bug. Either way you proposed to address this would be fine, so let's just roll with the one you implemented.
In Source/Editor/DualityEditor/Forms/ObjectRefSelectionDialog.Designer.cs https://github.com/AdamsLair/duality/pull/615#discussion_r162023131:
// columnName// this.columnName.Header = "Name"; this.columnName.MaxColumnWidth = 300; this.columnName.MinColumnWidth = 80; this.columnName.Sortable = true;
- this.columnName.SortOrder = System.Windows.Forms.SortOrder.Ascending;
Why was sorting by name disabled?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AdamsLair/duality/pull/615#pullrequestreview-89407018, or mute the thread https://github.com/notifications/unsubscribe-auth/AMWOAv-d2D8AFq-kyVsep8RmT5HxI6Nhks5tLdpygaJpZM4ReCV3 .
ilexp
Summary
Implemented #606. The implementation follows almost exactly as described in the issue text. There is one decision I made that is debatable, however.
The ReferenceNode class has three Reference properties that had public get/setters. With the edits I made, the Image property is assigned within the inspector based on the value of the Reference property passed into the constructor. If these properties were left with public setters, some later code could set these Reference properties to a new object that could need a different image than the one originally set in the constructor. The simplest solution was to make the setters private. Now, future uses of this class avoid a possible bug.
The alternative is to change the getter/setters to use private backing fields and to update the Image whenever the a Reference property changes. This would certainly work, but just seemed like more work than is necessary right now. If requested, I can implement that.