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

Scenario is complex, confusing, and causes errors #3350

Open
tig opened this issue Mar 25, 2024 · 2 comments
Open

Scenario is complex, confusing, and causes errors #3350

tig opened this issue Mar 25, 2024 · 2 comments
Labels
Milestone

Comments

@tig
Copy link
Collaborator

tig commented Mar 25, 2024

I over-engineered Scenario and tried to be both too helpful to devs and too cute. My intent was to make it super easy to create new scenarios. I thought it would be neat if Scenario developers only had to write a Setup override and the "default" logic in Init and Run would do magic for them. I also had UICatalog call Application.Shutdown when the Scenario exited instead of forcing Scenario authors to do this themselves.

This was a bad idea for these reasons:

  1. New developers looking at scenarios as examples don't see what ones supposed to do to setup/teardown a Terminal.Gui app. They don't see the proper way: Call Init, and ensure it's bracketed by a Shutdown, Call Run and make sure the Toplevel used gets disposed.
  2. It led to un-needed complexity in 4 overrides: Init, Setup, Run, and RequestStop. This is confusing to devs and a survey of the existing scenarios shows this confusion.
  3. It makes devs think that Application.Top can't be the actuall main view for the scenario, because Init, by default creates Win and adds it to Top.

My proposal:

  • Simplify Scenario to have only ONE virtual metods: Main. Developers should treat this just like the standard C# console app int Main (string [] args) method.
  • Change Init to NOT reset ConfigurationManager. This way whatever was configured in UI Catalog will naturally just pass onto the Scenario.
  • Change all Scenarios to merge their Init, Setup, and Run overrides into Main.

The Generic Scenario.Main will look like this:

    public override void Main ()
    {
        // Init
        Application.Init ();

        // Setup - Create a top-level application window and configure it.
        Window appWindow = new ()
        {
            Title = $"{Application.QuitKey} to Quit - Scenario: {GetName ()}",
        };

        var button = new Button { X = Pos.Center (), Y = Pos.Center (), Text = "Press me!" };
        button.Accept += (s, e) => MessageBox.ErrorQuery ("Error", "You pressed the button!", "Ok");
        appWindow.Add (button);

        // Run - Start the application.
        Application.Run (appWindow);
        appWindow.Dispose ();

        // Shutdown - Calling Application.Shutdown is required.
        Application.Shutdown ();
    }

Originally posted by @tig in #3339 (comment)

@tig tig changed the title Here's my thinking on Scenario. I'll either create a new issue/PR to address this or do it here, depending... Scenario is complex, confusing, and causes errors Mar 25, 2024
@tig tig added the bug label Mar 25, 2024
@tig
Copy link
Collaborator Author

tig commented Mar 25, 2024

I will enable this by adding Main and testing with a few Scenarios in #3339 but will not fix all scenarios there (to minimize churn).

@tig
Copy link
Collaborator Author

tig commented Apr 15, 2024

I added [ObsoleteAttribute ("This method is obsolete and will be removed in v2. Use Main instead.", false)] to the Scenario fields that will be deleted. Once those are gone from build-spew this Issue can be closed.

@tig tig added this to the V2 Beta milestone May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Status: 🏗 Approved - In progress
Development

No branches or pull requests

1 participant