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

s.makeGui server window broken in master #2202

Closed
jamshark70 opened this issue Jun 24, 2016 · 12 comments
Closed

s.makeGui server window broken in master #2202

jamshark70 opened this issue Jun 24, 2016 · 12 comments
Assignees
Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: class library SC class library severe
Milestone

Comments

@jamshark70
Copy link
Contributor

broken-server-window

Server is running. IDE server panel is updating. Server window is not updating.

I guess the new server status watcher is not finished yet?

@jamshark70 jamshark70 added bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: class library SC class library labels Jun 24, 2016
@jamshark70
Copy link
Contributor Author

https://github.com/supercollider/supercollider/blob/master/SCClassLibrary/Common/Control/ServerStatus.sc#L140

                    this.updateRunningState(true);
                    this.changed(\counts);

It's the Server object, not the ServerStatusWatcher, that should be calling changed.

@telephon
Copy link
Member

My refactored status watcher hasn't been merged, @crucialfelix wanted to be careful and incremental. I am now not sure, however, what is the best way to proceed.

@crucialfelix
Copy link
Member

Post a PR with a bug fix please.

That's the whole with huge unreadable PRs - even with spending hours with it something like that gets through. I'm not sure it it was something I missed or something in the original PR.

@crucialfelix crucialfelix added this to the 3.8 milestone Jun 28, 2016
@telephon
Copy link
Member

@crucialfelix I can try, but I don't think I can reimplement the whole recording pause-update-etc. thing in the IDE. It was already over my head, and I won't be able to do it all over. I understand that the PR was too big, but it just became that way and I had no way of simplifying it.

I guess better to just throw it away, even if it was several day's work.

@crucialfelix
Copy link
Member

Isn't the bug reported above just one small missing change ? It isn't getting the status.

The whole recording pause-update thing is separate. The rest of what you did is still waiting in that old PR to be cherry picked into a new smaller PR for each feature. If you (or me or somebody) starts from master now, that person can see what things are already merged in. I basically just did the first feature and then stopped.

I'll try to look at the issue above, but I'm pretty busy right now.

@telephon
Copy link
Member

OK, yes I see.

@telephon
Copy link
Member

telephon commented Jul 1, 2016

Is this fixed? I can't reproduce on the current master.

the only thing that seems missing is that the recording little R field in the IDE GUI doesn't update when you start and stop recording with the s.makeGui server window.

@nhthn
Copy link
Contributor

nhthn commented Jul 1, 2016

I can reproduce. In particular, avg CPU, number of synths, etc. aren't displaying.

@telephon
Copy link
Member

telephon commented Jul 1, 2016

Ah ok, I can reproduce this, too. I didn't look at them, because I misunderstood the issue description.

@jamshark70
Copy link
Contributor Author

Apologies for vagueness in the issue description. I had thought it would be clear from the screenshot that it should be inconsistent to have a server GUI showing "running" status while all the data fields are empty. Then again, I guess nobody is using the server GUI anymore so it may have been forgotten how it works.

@telephon
Copy link
Member

telephon commented Jul 2, 2016

No problem! Yes, I also think it would be better to make a new GUI once the refactoring is done. If it is OK even with the recording quirk, this would fix it.

@gusano
Copy link
Member

gusano commented Jul 8, 2016

@jamshark70 your change seems to fix it. Will submit a PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: class library SC class library severe
Projects
None yet
Development

No branches or pull requests

5 participants