BUG Report: ValueTree::getPropertyAsValue::refersToSameSourceAs

General JUCE discussion
1

Hi @juceteam

I found a bug using Value::refersToSameSourceAs with ValueTrees. Basically:

valueTree.getPropertyAsValue("someid").refersToSameSourceAs(valueTree.getPropertyAsValue("someid"));

returns false – which I think should not be the case. As far as I can see it can be easily fixed by added equality operations to ValueTreePropertyValueSource checking the valueTree and property members.

Thanks!

2

That’s consistent with the rest of the Value interface. Values only reference the same underlying object if you use the referTo method, or the copy constructor.

3

Hm, I’d argue that just by reading the code line I posted it’s pretty clear that the current implementation doesn’t make any sense.

And making the proposed change doesn’t change anything about the consistency. In that sense, the problem isn’t in the implementation of Value, its in ValueTree::getPropertyAsValue. I think this method should somehow ensure that the refersToSameSourceAs returns true if it is the semantically the same source. I don’t see any up side on the way it is currently as it is highly unexpected.

4

This is the consistency that would be broken:

var x { "x" };
Value a1 { x };
Value a2 { x };
jassert (! a1.refersToSameSourceAs (a2));

ValueTree vt { "vt", { { "prop", "val" } }};
auto b1 = vt.getPropertyAsValue ("prop", nullptr);
auto b2 = vt.getPropertyAsValue ("prop", nullptr);
jassert (! b1.refersToSameSourceAs (b2));
5

I think this is kind of comparing apples to oranges:

  var x { "x" };
  Value a1 { x };
  Value a2 { x };
  XCAssertTrue (! a1.refersToSameSourceAs (a2));
  
  a1.setValue("y");
  XCAssertTrue(a1.getValue() == a2.getValue()); // fails

  ValueTree vt { "vt", { { "prop", "val" } }};
  auto b1 = vt.getPropertyAsValue ("prop", nullptr);
  auto b2 = vt.getPropertyAsValue ("prop", nullptr);
  XCAssertTrue (! b1.refersToSameSourceAs (b2));
  
  b1.setValue("diff val");
  XCAssertTrue(b1.getValue() == b2.getValue()); // passes

The first example is expected to refer to different “sources” as changing Value a1 does not propagate any kind of updates or changes to Value a2 (a2 is still “x”).

But as the “source” of the second example is the ValueTree changing the Value b1 also changes the Value of b2. And I think this is exactly the behaviour that should be reflected by refersToSameSourceAs. I (as a user) don’t really care how that works implementation wise (e.g. is it really the same ValueSource object).

6

I still don’t think we should special-case the behaviour for ValueTree references. The implementation would also be horrible.

How are you using your ValueTree where this functionality is important?

7

In general: I’m trying to listen on property changes with a variable id. Simplified, that would look like:

Value v = valueTree.getPropertyAsValue(variableIdentifier); // stored as a member var
v.addListener(this);  // in ctor

After the value getting passed along a few times I have to check if I’m getting the correct value. I don’t see a possibility to check what specific property id I’m looking at through the Value interface.

I think using a virtual function in ValueSource would be a perfectly fine choice. The default implementation would return this == &other and the ValueTreeSource can implemented it (after a dynamic_cast check) with property == other.property

8

Why not listen to the ValueTree itself, or a sub-tree containing just your property?

A virtual function that imposes dynamic casts on anyone that might subclass ValueSource is not a pleasant way to go.

9

This decision was made because ValueTree lacks the possibility of passing a const version of it. Changing that is not an option for me. I can see how the dynamic_cast is not ideal… But I think how the interface is designed currently, it just doesn’t make semantically sense (as posted in the code above).

10

@t0m I’ve given it some thought and I think there would be an alternative to the dynamic_cast I proposed earlier. When all Values from getPropertyAsValue would get the same instance to one ValueTreePropertyValueSource you would get away with one additional virtual function call. The ValueSource would be given a function to check if a different ValueSource is actually pointing the same internal source after checking that the pointers are the same. The default implementation would be to straight return true and the ValueTree implementation can override the call to return true if valueTree and propertyId match.

This one and only ValueTreePropertyValueSource instance could live inside the ref-couted ValueTree object.

11

Why not just wrap Value in a custom class that fit your need (that i didn’t really understand TBH)?
I almost always decorate juce::Value::Listener and juce::Value::ValueSource to perfectly match my requirements. Sorry for hijacking your discussion.

12

Its ok, thanks for the suggestion! I was also thinking of doing that but I wouldn’t gain any benefit from doing so. juce::Value already provides me with exactly what I need, a setter, a getter and the possibility to listen for changes. The only upside would the type safety I guess.

But I think this all just covers the fact, that I’d like to be able to check if two values point to the same source and this is not possible even though there is a method of claiming to do that :slight_smile:

13

Personally i use templated wrappers to check that (at run-time sadly).
And with time it appears that i add more features that make the raw juce::Value unappeal anymore.

Anyway i don’t have any opinion about you original question/suggestion. I guess that ValueTree is the kind of master piece that is hard to change without pain!

1 Like