XR3Player icon indicating copy to clipboard operation
XR3Player copied to clipboard

Creating visualizer objects

Open HelgeStenstrom opened this issue 6 years ago • 15 comments

It seems to me that it's currently not possible to create an instance of Oscilloscope, without also creating an instance of Polyspiral, and vice versa. I think that's unfortunate. Am I right, and can that dependency be broken?

Apart from these two classes, the same dependency is between instances of all classes created at lin 24-28 of VisualizerDrawer.

HelgeStenstrom avatar Jun 20 '19 22:06 HelgeStenstrom

Ouh i have to see the code to answer you, actually i think you can create a Polyspiral without creating an Oscilloscope, can you send me the code that is proving the opposite :)?

goxr3plus avatar Jun 21 '19 07:06 goxr3plus

Look at the constructors for these classes:

	public Polyspiral(final VisualizerDrawer visualizerDrawer) {
		this.visualizerDrawer = visualizerDrawer;
	}
	public Oscilloscope(VisualizerDrawer visualizerDrawer) {
		this.visualizerDrawer = visualizerDrawer;
	}

and the beginning of VisaulizerDrawer:

public class VisualizerDrawer extends VisualizerModel {
	private final Oscilloscope oscilloscope = new Oscilloscope(this);
	private Polyspiral polySpiral = new Polyspiral(this);
	protected final Sierpinski sierpinski = new Sierpinski(this);
	private final JuliaSet juliaSet = new JuliaSet(this);
	private Sprites3D sprite3D = new Sprites3D(this, Shape3D.SPHERE);

When you create Oscilloscope or PolySpiral, you need to provide a VisualizerDrawer as argument. When you create a VisualizerDrawer, both Oscilloscope and PolySpiral will be created.

HelgeStenstrom avatar Jun 22 '19 11:06 HelgeStenstrom

Aouuuu now i catch you, i was Junior developer and didn't knew well refactoring concepts back then. I see... your point.

goxr3plus avatar Jun 23 '19 09:06 goxr3plus

@HelgeStenstrom Please run the application after refactorings because it ,may break , i am trying to run it after the merge requests and now i am getting full of errors , i will fix but be careful :

idea64_2019-06-24_17-52-15

goxr3plus avatar Jun 24 '19 14:06 goxr3plus

I didn’t run it. Your problem above looks like a compile time problem, and I doubt that it’s caused by my changes. At least not by one particular change, but perhaps if merge conflicts were not properly solved.

HelgeStenstrom avatar Jun 25 '19 05:06 HelgeStenstrom

It was due to making <Boolean> to <> on some services. I fixed it It was something new for me :)

goxr3plus avatar Jun 25 '19 06:06 goxr3plus

I have made a branch where Oscilloscope and its siblings can now be instantiated independently of each other. They are no longer owned by VisusalizerDrawer, but by a new class GadgetOwner.

This might be a step towards a better structure, but there are still issues. The instances are now created upon initialization of PaintService. (The GadgetOwner object is created there and then, with its owned objects). It would be better to inject the GadgetOwner into the PaintService object.

GadgetOwner is not a good name, but it will do for now.

Of the five objects owned by GadgetOwner, I have tested four. I didn't manage to visualize using the JuliaSet from the GUI, but I think it would work.

Take a look at https://github.com/HelgeStenstrom/XR3Player/tree/refactorOscilloscope

I can make a pull request from it, if you want, but it's not ready for merging.

HelgeStenstrom avatar Jul 08 '19 21:07 HelgeStenstrom

I fooled myself: my refactoring did not remove the cross-dependence between VisualizerDrawer and the "gadgets". I just moved the point where these items are created. I have to think a bit more.

HelgeStenstrom avatar Jul 09 '19 14:07 HelgeStenstrom

Again I have to correct myself. The refactoring did help. That is because the creation of Oscilloscope and siblings are done at creation of Visualizer, but Oscilloscope only needs a reference to an VisualizerDrawer. So in a unit test, I can create a VisualizerDrawer, and pass it to a new Oscilloscope. No Visualizer will be created, and therefore no siblings of Oscilloscope. Right now the creation takes place in the constructor of PaintService, but that doesn't feel like a natural place. Better to have it in Visualizer. In practice, there is not much difference.

HelgeStenstrom avatar Jul 09 '19 15:07 HelgeStenstrom

@HelgeStenstrom Give it a pull request i like your idea, we will make a library from iy and completely remove it from XR3Player so we will use the library :)

goxr3plus avatar Jul 09 '19 16:07 goxr3plus

I will will finish it for you, let me see how it looks like :)

goxr3plus avatar Jul 09 '19 16:07 goxr3plus

I want to make XR3Player professional product like Traktor and VirtualDJ, it's not long from there and in some places it has more features.

Imagine it having 20. 000 stars one day and you are a contributor :)

goxr3plus avatar Jul 09 '19 16:07 goxr3plus

If I understand correctly, once the player is started, we have 5 instances of Oscilloscope, 5 instances of JuliaSet, and so on. How many of these may at most be visible at the same time? I don’t know the lifetime of these objects. Do they live until the application is closed? For the object called player0 (if I remember correctly), is there a need to show both Oscilloscope and Sierpinsky or JuliaSet simultaneously? If not, they can be created when they need to be shown, and left to garbage collection when they don’t. On the other hand, I don’t think they take much resources. But I care about understandable code.

HelgeStenstrom avatar Jul 09 '19 21:07 HelgeStenstrom

We have 3 XR3PlayerControllers (each one representing a player), each one containing a Visualizer which has 1 instance of all the above mentioned before .

This below is an XR3PlayerController class :

image

goxr3plus avatar Jul 11 '19 13:07 goxr3plus

You can see 3 of them in the application . Inside it contains the visualizer the djdisc etc etc etc all the objects .

goxr3plus avatar Jul 11 '19 13:07 goxr3plus