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

Observable Configs #1709

Open
emanuele3d opened this issue May 2, 2015 · 9 comments
Open

Observable Configs #1709

emanuele3d opened this issue May 2, 2015 · 9 comments
Assignees
Labels
Category: Performance Requests, Issues and Changes targeting performance Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc.

Comments

@emanuele3d
Copy link
Contributor

emanuele3d commented May 2, 2015

A small boost in performance in per-frame or per-update code could be achieved if instances of a class didn't have to look at config values unless they get notified that values have changed. The code too would also tend to be a little leaner.

For this purpose Configs could be made to implement an Observable interface (i.e. subscribe/unsubscribe methods - whatever is most common in Java or more specifically in Terasology) to register instances implementing an Observer interface and interested in being notified of a Config's changes.

@emanuele3d
Copy link
Contributor Author

I thought of a number of Observable/Observer styles:

  • the config informs the observers it has changed, but it's up to the observers to find how.
  • the config informs the observers that a number of its parameters have changed, i.e. providing a set of strings. The observers can then use those strings to see if they are interested in fetching any of the changed values.
  • the config informs the observers providing a map that the observers can browse to immediately obtain changed values.

And potentially the config might even keep track of what an observer is interested in, notifying it only if the specific parameters an observer is interested in have changed. I don't know if we'd want that much responsibility in the Configs though.

Thoughts?

(p.s. would this be best discussed in the forum instead? I figured it's a bit of a feature request and opening an issue might not be entirely inappropriate)

@Cervator Cervator added the Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc. label May 2, 2015
@Cervator
Copy link
Member

Cervator commented May 2, 2015

It might be more of a forum thing, yeah, but it sounds reasonable enough and might get more bites here than in the forum. Hard to say.

Pinging @immortius @flo @MarcinSc for ideas

@Cervator Cervator added the Category: Performance Requests, Issues and Changes targeting performance label May 2, 2015
@immortius
Copy link
Member

I would recommend using the standard
http://docs.oracle.com/javase/7/docs/api/java/beans/PropertyChangeSupport.html
approach.

On 3 May 2015 at 07:21, Rasmus Praestholm notifications@github.com wrote:

It might be more of a forum thing, yeah, but it sounds reasonable enough
and might get more bites here than in the forum. Hard to say.

Pinging @immortius https://github.com/immortius @flo
https://github.com/flo @MarcinSc https://github.com/MarcinSc for ideas


Reply to this email directly or view it on GitHub
#1709 (comment)
.

@MarcinSc
Copy link
Contributor

MarcinSc commented May 3, 2015

What about config sending an exception on the world entity (or a separate "config" entity), since we are already event driven...

@msteiger
Copy link
Member

msteiger commented May 3, 2015

I use an Observable interface in WorldViewer to notify listeners when - for example - the camera has moved. It is similar to java.util.Observable, but supports a generic type.

In contrast to Java's property change listeners it can be implemented by lambda expressions and you don't need use String constants for identification.

I think it could make sense using that instead of the in-game event system, because the linking between Observer and Observable is fairly explicit and it works without the entity system (e.g. in the main menu).

I'm planning to move some of the core functionality of WorldViewer over the next days here to improve the preview screen, and Observable is part of that.

@immortius
Copy link
Member

Hmm, the lambda support is a good point, and I agree events are not
suitable for config. However PropertyChangeListener does support lambda
expressions.

I think the PropertyChangeSupport approach is a better fit in this space
because:

  • Settings are likely to change individually. Observable is better suited
    for batch changes.
  • The notification process is built into the setter. Observable relies on
    the changer calling notify.

Yes, the PropertyChangeSupport requires string names for each property -
this is to support telling listeners which property has changed. Observable
cannot do this as easily.

Threading is a concern in both cases.

I won't deny that PropertyChangeSupport is a bit annoying to integrate,
updating each setter and add string constants for each one. Using a code
template or macro works well. It is best when used by code generators.
On 03/05/2015 4:56 PM, "Martin Steiger" notifications@github.com wrote:

I use an Observable interface in WorldViewer to notify listeners when -
for example - the camera has moved. It is similar to java.util.Observable,
but supports a generic type.

In contrast to Java's property change listeners it can be implemented by
lambda expressions and you don't need use String constants for
identification.

I think it could make sense using that instead of the in-game event
system, because the linking between Observer and Observable is fairly
explicit and it works without the entity system (e.g. in the main menu).

I'm planning to move some of the core functionality of WorldViewer over
the next days here to improve the preview screen, and Observable is part
of that.


Reply to this email directly or view it on GitHub
#1709 (comment)
.

@Cervator
Copy link
Member

Bump - does this relate to the work in #1843 / #1844 ? Thought I'd link in case of overlap.

@msteiger
Copy link
Member

I don't think so, but this issue here is next up on my todo list.

@emanuele3d
Copy link
Contributor Author

emanuele3d commented Jul 19, 2016

I'm currently working on part of this. I just went through the RenderingConfig and made both the instance itself and almost all its properties observable. I've eventually settled for the PropertyChangeSupport-based approach as there was already one usage of it in the class. I will go through the RenderingDebugConfig and do the same. I aim to submit a PR in the next 24/48 hours.

Tagging @tdgunes and @indianajohn as it might be interesting for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Performance Requests, Issues and Changes targeting performance Topic: Architecture Requests, Issues and Changes related to software architecture, programming patterns, etc.
Projects
None yet
Development

No branches or pull requests

5 participants