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

Auto Mode #459

Open
wants to merge 2,048 commits into
base: main
Choose a base branch
from
Open

Auto Mode #459

wants to merge 2,048 commits into from

Conversation

alejot127
Copy link
Collaborator

Still working on some kinks, but this is my general approach. Threads in Kotlin are new to me as well, so maybe there's a better way to do this + I may be doing it way wrong

xian and others added 30 commits March 4, 2022 13:45
…igs.

* Incorporate default fixture and transport configs into brain controllers.
* Clean up UI.
* Backfill some tests.
…fixture type is.

* Automatic DMX assignments.
* Rename FixtureMappingData.deviceType to fixtureType.
* Catch and display fixture config errors instead of blowing up.
* Honcho scene uses auto DMX allocation.
In-app controller configuration
Swap simulator and app positions for small devices.
Apply entity transformation to its pixel locations
Matrices used to be stored as an object with an "elements" array; now they're just an array.
@alejot127 alejot127 requested a review from xian June 28, 2022 19:28
build.gradle.kts Outdated Show resolved Hide resolved
src/commonMain/kotlin/baaahs/show/live/AutoMode.kt Outdated Show resolved Hide resolved
private suspend fun playWithButtons() {
val controls: List<OpenButtonControl> = openShow.allFilterButtonControls
logger.info { "Controls " + controls.joinToString { it.gadget.title } }
val random = Random(indexSeed++)
Copy link
Member

Choose a reason for hiding this comment

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

You could store the Random as a private var if you want, no need really to reinitialize it every time, you'll keep getting pseudo-random numbers forever.

Fwiw I don't usually bother with creating explicit Randoms unless unit tests interact with it, in which case they're awesome.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah not sure how to get truly random results since the seed is always the same, so the auto mode will always pick the same number sequence :/

One solution is to get the current time in seconds as the seed to always get diff results, but I couldn't reference this for some reason: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.system/get-time-millis.html

var control: OpenButtonControl
while (autoModeOn) {
control = controls.random(random)
// This doesn't update the UI :(
Copy link
Member

Choose a reason for hiding this comment

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

Huh, yeah, janky. It definitely seems like click() ought to do what you want.

There are a couple ways the button might be enabled/disabled:

  • when a user directly clicks on it. In this case, ButtonControlView calls handleToggleClick(), which calls button.click(), but also onShowStateChange(), which ultimately calls ShowManager.Facade.onShowStateChange(), which re-renders the page.
  • when a user on another client clicks it. In this case, I'm not quite sure exactly how the UI gets updated; I think it's something like this: the switch gadget gets an update over pubsub, then notifies its observers, which include GadgetManager, which is itself being observed by StageManager, which ... somehow magically causes a re-render. I'm missing something there clearly. I'll look more closely in a few.

In short, this is all kinda a spaghetti-tastic mess. Thinking about how to improve matters.

logger.info { "Stop Called" }
autoMode = false;
CoroutineScope(Dispatchers.Unconfined).launch {
gadgetWizard?.cancelAndJoin()
Copy link
Member

Choose a reason for hiding this comment

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

Nice :-)

@@ -6,6 +6,7 @@ import kotlinx.serialization.Serializable
@Serializable
data class UiSettings(
val darkMode: Boolean = true,
val autoMode: Boolean = false,
Copy link
Member

Choose a reason for hiding this comment

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

I think autoMode makes more sense as show state rather than client config state. UiSettings is meant for config on the local client, and doesn't get synched to other open clients, but you probably want autoMode to show the same value on every open client. Otherwise there'd be no way to stop an unattended iPad from monkeying with your show.

Thinking about it a little more, implementing it that way actually raises some weird issues: it implies that some single entity needs to be in charge of the monkeying, which makes me think it really should be happening on the server. Further, if there's no client currently connected I think we'd still like monkeying to happen potentially, another vote in favor of the server doing it.

... which would require rethinking our approach a little bit. Maybe it makes sense to get it working as-is since time grows short. 🤷‍♀️

@@ -37,6 +37,13 @@ class OpenPatch(
}
}

val inEqualsOut: Boolean
Copy link
Member

Choose a reason for hiding this comment

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

Any insight into why isFilter seemed to be behaving badly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So isFilter might be fine, this was before I tried to fetch controls from the openlayout, which doesn't include the "backdrop" control group.

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay, I bet it doesn't traverse into children.

{"runningShowPath":"PastureBedtime.sparkle","runningScenePath":"Arttitude.scene", "autoModeOn": true }
Copy link
Member

Choose a reason for hiding this comment

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

Not sure this should be a persisted server setting.

If Pinky restarts, maybe we start in auto mode==true until we get some client interaction? Anyway, not a big deal either way, but it doesn't feel like important state to save between restarts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oic

@@ -126,6 +126,11 @@ class OpenButtonControl(
isPressed = !isPressed
}

fun forceClickChange() {
click()
switch.changed()
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, good workaround. Could you drop in a comment saying it's a workaround for listeners not being quite right?

Copy link
Member

Choose a reason for hiding this comment

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

(Eventually I'd like to clean up the listeners so it does the right thing on its own.)

@@ -37,6 +37,13 @@ class OpenPatch(
}
}

val inEqualsOut: Boolean
Copy link
Member

Choose a reason for hiding this comment

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

Ah okay, I bet it doesn't traverse into children.

@@ -205,6 +225,8 @@ class StageManager(
updateRunningShowPath(file)

notifyOfDocumentChanges(fromClientUpdate)

autoModeWizard.setShow(showRunner!!.openShow)
Copy link
Member

Choose a reason for hiding this comment

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

You can get this behavior by giving AutoMode a ShowMonitor:

class AutoMode(showMonitor: ShowMonitor) {
    init {
        showMonitor.addObserver { setShow(it.openShow) }
    }

    ...
}

@@ -12,6 +13,9 @@ data class ClientData(
val fsRoot: Fs.File
)

@Serializable
class AutoModeCommand(val autoModeState: AutoModeState)
Copy link
Member

Choose a reason for hiding this comment

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

I think you don't need this, just the AutoModeState channel.

logger.info { "Successful subscribe then publish" }
val newMode = newAutoMode == AutoModeState.On
val newSettings = uiSettings.copy(autoMode = newMode)
updateUiSettings(newSettings, saveToStorage = true)
Copy link
Member

Choose a reason for hiding this comment

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

I'd just make this a property on the facade rather than putting it in UiSettings, cuz it doesn't need to be persisted across browser reloads — the one source of truth should be the pubsub channel.

var autoModeState = AutoModeState.OFF
pubSub.subscribe(Topics.autoMode) { newAutoMode ->
    automodeState = newAutoMode
    facade.notifyChanged()
}

@alejot127 alejot127 changed the title WIP: Auto Mode Auto Mode Jul 28, 2022
@xian
Copy link
Member

xian commented Aug 10, 2022

Sorry I'm so slow! Failing tests here, I can help look a little later for clues…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To do
Development

Successfully merging this pull request may close these issues.

3 participants