Skip to content

Wip test monitors docked - #244

Merged
TheBizzle merged 1 commit into
masterfrom
wip-test-monitors-docked
Jun 20, 2015
Merged

Wip test monitors docked#244
TheBizzle merged 1 commit into
masterfrom
wip-test-monitors-docked

Conversation

@mrerrormessage

Copy link
Copy Markdown
Contributor

This is for Galapagos changes which enable NetLogo/Tortoise#12 to work in Galapagos. I do not expect this PR to be merged without serious modification, but I wanted to have something that works to start the discussion off on the right foot.

Within Galapagos, the UI depends on the widgets, and tortoise code depends on the UI. This hasn't been a problem until now because we have simply passed the widgets to the browser as part of a separate data structure. However, as part of Tortoise#12, widgets are now compiled with the rest of the model and put into a variable: var widgets = [...], so that the UI cannot be constructed before the model is evaluated. However, some code in the model assumes that the UI has been setup prior to it's evaluation, which causes problems. I'm pretty convinced, based on NetLogo-JVM's use of events in GUI loading code, that the problem is at least partially inherent to NetLogo.

There may be a number of solutions which I am not clever enough or familiar enough with the code to imagine, and I would really appreciate hearing them. I came up with these:

  1. Continue to have a separate "widgets" object/array which is used to reconstitute the UI before the model is eval'ed.
  2. Create some sort of event queue which temporally decouples model evaluation from UI initialization. This is the approach I took here for the plot events in particular, creating a proxy object which queued up plot events. The PlotOpsManager and PlotOpsProxy listen as the model is loaded and replay the events once the UI has been constructed.
  3. Define a function in the model that is called after the model has been eval'ed which performs model setup vis-脿-vis the GUI. Right now, we perform model setup by sticking a bunch of commands at the end, but there's no reason those commands couldn't live in a function which gets called after eval has read in the model and the GUI has been setup. This would require some modification in Tortoise, but if we're going this route, the sooner we make those modifications the easier it will be.

@TheBizzle , @qiemem, please comment and let me know your thoughts. Unless we can come to a solution before then, I hope we can discuss this (at least briefly) in CCL-Devel tomorrow.

@mrerrormessage mrerrormessage self-assigned this Jun 9, 2015
@SethTisue

Copy link
Copy Markdown
Contributor

I can confirm this has always been a source of trickiness in the JVM NetLogo codebase. The Code and Interface tabs are inherently mutually dependent. Code in the Code tab may refer to sliders, switches, and input boxes in the Interface tab, and the widgets in the Interface may contain code that references code in the Code tab.

Comment thread app/models/json/JsonConverter.scala Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

elems.toMap.mapValues(toTortoise)

@mrerrormessage
mrerrormessage force-pushed the wip-test-monitors-docked branch from e6a74c9 to b23d603 Compare June 19, 2015 15:22
@mrerrormessage

Copy link
Copy Markdown
Contributor Author

Comment addressed. Once the version of tortoise is final, we'll need to update build.sbt.

@TheBizzle

Copy link
Copy Markdown
Member

This looks good. It will be ready for merge once the Tortoise SHA is finalized.

@mrerrormessage
mrerrormessage force-pushed the wip-test-monitors-docked branch from b23d603 to 2c1f0df Compare June 19, 2015 20:18
@TheBizzle
TheBizzle merged commit 2c1f0df into master Jun 20, 2015
@TheBizzle
TheBizzle deleted the wip-test-monitors-docked branch June 20, 2015 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants