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

sclang: refactor server menu #3870

Merged
merged 1 commit into from
Feb 2, 2019
Merged

sclang: refactor server menu #3870

merged 1 commit into from
Feb 2, 2019

Conversation

scztt
Copy link
Contributor

@scztt scztt commented Jul 13, 2018

This fixes issues where server menu flashes when updating because we were detroying the entire thing to rebuild it - now we just incrementally update.

This fixes issues where server menu flashes when updating because we were detroying the entire thing to rebuild it - now we just incrementally update.
@patrickdupuis
Copy link
Contributor

This is causing some breakage for me:

ERROR: Primitive '_QObject_InvokeMethod' failed.
Failed.
RECEIVER:
Instance of Menu {    (0x2588618, gc=4C, fmt=00, flg=00, set=06)
  instance variables [34]
    qObject : RawPointer 0x1ca3ad0
    finalizer : instance of Finalizer (0x2637998, size=2, set=1)
    virtualSlots : nil
    wasRemoved : false
    font : nil
    resize : Integer 1
    alpha : Float 1.000000   00000000 3FF00000
    decorator : nil
    layout : nil
    userCanClose : true
    deleteOnClose : true
    action : nil
    mouseDownAction : nil
    mouseUpAction : nil
    mouseEnterAction : nil
    mouseLeaveAction : nil
    mouseMoveAction : nil
    mouseOverAction : nil
    mouseWheelAction : nil
    keyDownAction : nil
    keyUpAction : nil
    keyModifiersChangedAction : nil
    keyTyped : nil
    focusGainedAction : nil
    focusLostAction : nil
    dragLabel : nil
    beginDragAction : nil
    canReceiveDragHandler : nil
    receiveDragHandler : nil
    toFrontAction : nil
    endFrontAction : nil
    onClose : nil
    onResize : nil
    onMove : nil
}
CALL STACK:
	MethodError:reportError
		arg this = <instance of PrimitiveFailedError>
	Nil:handleError
		arg this = nil
		arg error = <instance of PrimitiveFailedError>
	Thread:handleError
		arg this = <instance of Thread>
		arg error = <instance of PrimitiveFailedError>
	Object:throw
		arg this = <instance of PrimitiveFailedError>
	Object:primitiveFailed
		arg this = <instance of Menu>
	ArrayedCollection:do
		arg this = [*4]
		arg function = <instance of Function>
		var i = 0
	< FunctionDef in Method Meta_MainMenu:prUpdateServersMenu >
		var actions = [*4]
		var new = <instance of Event>
	< FunctionDef in Method Meta_MainMenu:prUpdateServersMenu >
		arg s = <instance of Server>
		var actionsList = nil
		var startString = nil
		var runningString = nil
		var defaultString = nil
	< FunctionDef in Method Set:do >
		arg item = <instance of Server>
	ArrayedCollection:do
		arg this = [*4]
		arg function = <instance of Function>
		var i = 0
	Set:do
		arg this = <instance of Set>
		arg function = <instance of Function>
		var i = 0
	Meta_MainMenu:prUpdateServersMenu
		arg this = <instance of Meta_MainMenu>
	Meta_MainMenu:initClass
		arg this = <instance of Meta_MainMenu>
	Meta_Class:initClassTree
		arg this = <instance of Meta_Class>
		arg aClass = <instance of Meta_MainMenu>
		var implementsInitClass = nil
	ArrayedCollection:do
		arg this = [*269]
		arg function = <instance of Function>
		var i = 222
	Meta_Class:initClassTree
		arg this = <instance of Meta_Class>
		arg aClass = <instance of Meta_Object>
		var implementsInitClass = nil
...
^^ The preceding error dump is for ERROR: Primitive '_QObject_InvokeMethod' failed.
Failed.
RECEIVER: Menu("Servers")

@patrickdupuis
Copy link
Contributor

@telephon can you explain why you approved this PR given my comment above?

@patrickdupuis
Copy link
Contributor

Does this PR depend on the fixes from #3871?

@scztt
Copy link
Contributor Author

scztt commented Jul 13, 2018

Does this PR depend on the fixes from #3871?
Yes, it does. I'll merge that into this branch so it passes travis.

@scztt scztt added the comp: Qt GUI sclang Qt components -- for IDE tickets, use "env: SCIDE" instead label Jul 13, 2018
@nhthn nhthn changed the base branch from develop to 3.10 July 15, 2018 19:05
@telephon
Copy link
Member

I approved of it because it fixes issues that scott discovered in his own code. I don't want to hold up patches to peoples own contributions. I didn't have this failure for some reason.

@patrickdupuis
Copy link
Contributor

@telephon try testing this out against 3.10 and see if you get the same breakage.

@telephon
Copy link
Member

now I get it, too.

I did a bit of digging (it is not so convenient that the error doesn't say what call actually failed), I found that it is here:

insertAction {
		|before, action|
		if (before.isKindOf(Number)) {
			before = this.actions[before]
		};

		^this.invokeMethod('insertAction', before, action.asMenuAction); // <--- this is probably wrong, because invokeMethod takes a boolean as third argument.
	}

@telephon
Copy link
Member

invokeMethod { arg method, arguments, synchronous = true;
		_QObject_InvokeMethod
		^this.primitiveFailed
	}

Copy link
Member

@telephon telephon left a comment

Choose a reason for hiding this comment

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

we need to also fix the bug in the invokeMethod call (see conversation).

@telephon
Copy link
Member

Ah here I found the fix, which is in develop: #3871

@nhthn nhthn added this to the 3.10 milestone Jul 17, 2018
@patrickdupuis
Copy link
Contributor

#3871 is now in 3.10. I no longer get the above error. Maybe we can re-awaken the CI?

@telephon
Copy link
Member

Maybe we can re-awaken the CI?

sorry, I don't understand the abbreviation.

@mossheim
Copy link
Contributor

continuous integration

@patrickdupuis
Copy link
Contributor

@telephon have your requests been met?

@patrickdupuis
Copy link
Contributor

@scztt told me over Slack that this PR is passing the CI, but that it doesn't show here. @telephon can you approve?

@patrickdupuis patrickdupuis modified the milestones: 3.10, 3.10.1 Sep 16, 2018
@patrickdupuis patrickdupuis modified the milestones: 3.10.1, 3.10.2 Jan 9, 2019
@nhthn nhthn self-assigned this Jan 27, 2019
@mossheim mossheim dismissed telephon’s stale review February 2, 2019 22:46

stale review, concerns addressed

Copy link
Contributor

@mossheim mossheim left a comment

Choose a reason for hiding this comment

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

lgtm. one small bug which I will address in a separate PR

@mossheim mossheim merged commit bffb9a3 into 3.10 Feb 2, 2019
@mossheim mossheim deleted the topic/refactor-server-menu branch February 2, 2019 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: Qt GUI sclang Qt components -- for IDE tickets, use "env: SCIDE" instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants