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

Add property viewer to DrawVisualiser #780

Merged
merged 33 commits into from Jun 16, 2017
Merged

Add property viewer to DrawVisualiser #780

merged 33 commits into from Jun 16, 2017

Conversation

luaneko
Copy link
Contributor

@luaneko luaneko commented Jun 3, 2017

Added PropertyDisplay which lists all public property values and some graphical changes like highlighting.
Also a commit which simply overrides some ToString()s for better property viewing.

paparony03 added 3 commits June 3, 2017 15:44
Also fixes #291
For better reading in PropertyDisplay
@@ -371,5 +371,7 @@ private void checkAudioDeviceChanged()
{
}
}

public override string ToString() => $@"AudioManager ({currentAudioDevice})";

This comment was marked as off-topic.

@@ -63,6 +63,8 @@ private int getAdditionIndex(T value)
return index;
}

public override string ToString() => $@"{GetType().Name} ({Count} items)";

This comment was marked as off-topic.

@Tom94
Copy link
Collaborator

Tom94 commented Jun 3, 2017

Looks very nice now! I have only a few more requests before I think this is ready for merging:

  • As long as the property viewer is disabled, no VisualisedDrawable should be highlighted.
  • Only the Drawable whose properties are shown should be highlighted; not the entire subtree.
  • When highlighting a VisualisedDrawable via right-click the property viewer should automatically become enabled if it wasn't before.
  • The "Up one parent" button shouldn't change the highlighted VisualisedDrawable.
  • Can you make properties flash if their value changes? That would be super useful to spot which part of the visualised drawable is active, and it can help pull attention to single-frame on-off toggles. Use Drawable.FlashColour for this and use a different colour from the one used to highlight hovered properties.

@peppy
Copy link
Sponsor Member

peppy commented Jun 4, 2017

  • Time and BoundingBox are still jumping around like crazy due to lack of rounding.
  • OnLoadComplete is always red, which would suggest it's getting fired every frame. I don't think this is the case.

if (viz.highlighted == this)
HighlightTarget?.Invoke(null);

else HighlightTarget?.Invoke(this);

This comment was marked as off-topic.

@Tom94
Copy link
Collaborator

Tom94 commented Jun 4, 2017

The property window gets enabled when re-targeting other drawables. That shouldn't happen. The properties window state should be totally independent from drawable selection. Only highlighting via right-click or toggling via the "Properties" button should change its state.


return AlmostEquals(value, rounded, acceptableDifference) ? rounded : value;
return rounded == value ? rounded : value;

This comment was marked as off-topic.

This comment was marked as off-topic.

@@ -53,7 +53,7 @@ public static double Round(double value, int numOfDigits = 7)

public static float Round(float value, int numOfDigits = 3)

This comment was marked as off-topic.

@peppy peppy changed the title Better visualizer Add property viewer to DrawVisualiser Jun 6, 2017
smoogipoo
smoogipoo previously approved these changes Jun 7, 2017
Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

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

There are more improvements I'd like to see to the draw visualiser but this is a good start 👍

{
if (args.Button == MouseButton.Right)
{
HighlightTarget?.Invoke(viz.Highlighted == this ? null : this);

This comment was marked as off-topic.

public void Collapse()
{
if (viz.Highlighted == this)
return;

This comment was marked as off-topic.

updateSpecifics();
if (Flow.IsPresent)
Collapse();

This comment was marked as off-topic.

@@ -30,7 +31,8 @@ public int Compare(VisualisedDrawable x, VisualisedDrawable y)

public Drawable Target { get; }

private readonly Box textBg;
private readonly Box background;
internal readonly Box highlightBackground;

This comment was marked as off-topic.

@@ -186,6 +269,26 @@ private VisualisedDrawable createVisualisedDrawable(VisualisedDrawable parent, D

return vis;
}

This comment was marked as off-topic.

@@ -159,6 +226,21 @@ private void runUpdate()

visualise(Target, targetDrawable);
}

This comment was marked as off-topic.


propertyDisplay.Add(
((IEnumerable<MemberInfo>)type.GetProperties(BindingFlags.Instance | BindingFlags.Public)) // Get all properties
.Concat(type.GetFields(BindingFlags.Instance | BindingFlags.Public)) // And all fields

This comment was marked as off-topic.

{
Depth = float.MinValue,
Depth = float.MinValue

This comment was marked as off-topic.

@peppy
Copy link
Sponsor Member

peppy commented Jun 13, 2017

@paparony03 you still alive?

Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

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

I think there are more code styling improvements that can be made, but I will do so a bit later.

@@ -1,391 +1,392 @@
<?xml version="1.0" encoding="utf-8"?>

This comment was marked as off-topic.

This comment was marked as off-topic.

protected override bool OnClick(InputState state)
{
Flow.Alpha = Flow.Alpha > 0 ? 0 : 1;
updateSpecifics();
if (Flow.IsPresent)

This comment was marked as off-topic.

{
get
{
return highlightBackground.Alpha > 0;

This comment was marked as off-topic.

}
}

internal class PropertyItem : Container

This comment was marked as off-topic.


namespace osu.Framework.Graphics.Visualisation
{
public class DrawVisualiser : OverlayContainer
{
private readonly TreeContainer treeContainer;
internal readonly TreeContainer TreeContainer;
internal VisualisedDrawable HighlightedTarget;

This comment was marked as off-topic.

@smoogipoo
Copy link
Contributor

Properties panel looks bad hiding when selecting a new drawable.

@peppy peppy merged commit 3dd7516 into ppy:master Jun 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants