Implement automation mechanism #78

Wiguwbe · 2023-08-18T18:33:40Z

(a bit of a fuss with rebase on #75 and got automatically closed, re-creating)

An external script can be used to automate the scheduling.

An event manager receives events from processes, pages and IO queue,
and dispatches them to the external script.

The external script then processes the events and returns a list of
actionable events (simulating clicks).

The game scene dispatches the received events to the respective receivers.

Main changes are:

Process, Page and IoQueue have static variables to keep a track of created instances (1)
Page is identified by its pair PID (owner ID) and IDX (index of page inside process)
event manager;
changes to provide events to event manager;
compile and execute an external script;
_on_click/_onClick is now public;
game's scene dispatches events from script before calling the update function

An example automated script is provided, mostly to explain the
exposed API, it has a (not so) simple example implementation.

(1). The Process/Page Managers only have lists of them, this needed (in my opinion) a mapping (int -> object), this also allows for them to be fetch from anywhere (maybe not a lot safe). (There are safer options, I may look into them in the future)

An external script can be used to automate the scheduling. An event manager receives events from processes, pages and IO queue, and dispatches them to the external script. The external script then processes the events and returns a list of actionable events (simulating clicks). The game scene dispatches the received events to the respective receivers. Main changes are: - event manager; - changes to provide events to event manager; - compile and execute an external script; - _on_click/_onClick is now public; - game's scene dispatches events from script before calling the update function An example automated script is provided, mostly to explain the exposed API, it has a (not so) simple example implementation.

Wiguwbe · 2023-08-18T18:55:34Z

src/game_objects/process.py

@@ -62,13 +65,34 @@ def starvation_level(self):
    def display_blink_color(self):
        return self._display_blink_color

+    def _update_blocking_condition(self, update_fn):


was this section recently removed? it may have been re-enabled in my rebase mess

It was not removed, but it was moved lower. So it appears to be duplicated now.

this duplicate was removed

it was previously changed to accomodate events coming from the automation script. as the way those events were handled changed, this change was left behind

Wiguwbe · 2023-08-19T09:30:47Z

src/game_objects/page_manager.py

 class PageManager(GameObject):
    _TOTAL_ROWS = 11
+    _NUM_RAM_COLS = 16


added class constant (and methods below) to be used from scene.Game to pass to the automated script

Maybe rename it to _NUM_PAGE_COLS or event just _NUM_COLS (which would be consistent with _TOTAL_ROWS) to make it clear that it's also for the rows of pages that are on disk?

Wiguwbe · 2023-08-19T09:31:56Z

src/game_objects/process.py

        if self.has_cpu:
            self.yield_cpu()
        else:
            self.use_cpu()

    def update(self, current_time, events):
+        # pylint: disable=too-many-statements


have a too-many-statements warning (51/50)
a refactor could/should take place, but I'm not confident enough to do it :)

I might disable this project-wide actually, I don't need pylint to be THAT judgemental!

Wiguwbe · 2023-08-19T09:32:36Z

src/main.py

@@ -13,6 +13,23 @@
 from game_info import TITLE
 from window_size import WINDOW_SIZE

+
+def compile_auto_script():


transformed into a function so that pylint doesn't complain about constants without uppercase

Wiguwbe · 2023-08-19T09:33:32Z

src/scenes/game.py

+        # for automation
+        self._script_callback = None
+        if self._script:
+            # pylint: disable=exec-used


pylint complains about exec being used, which in this implementation is necessary

Could you move lines 77 to 94 to another method?

Wiguwbe · 2023-08-19T09:34:44Z

src/scenes/game.py

+                        Process.Processes[event['pid']].on_click()
+                    elif event['type'] == 'page':
+                        Page.Pages[(event['pid'],event['idx'])].on_click()
+                except Exception as exc: # pylint: disable=broad-exception-caught


this broad exception catches any problem that may come from each event
(I had a few of indexError and valueError when testing my own script)

Wiguwbe · 2023-08-19T09:35:45Z

Merged from upstream and pylint should give a 10/10 now.
Some warning were disabled and explained above

plbrault

This is great overall! I hope I am not being too picky with my change requests. And sorry for causing you more work by introducing pylint in the meantime!

plbrault · 2023-08-19T15:36:35Z

README.md

@@ -36,6 +36,14 @@ pipenv run desktop
 pipenv run web
 ```

+**Run desktop version with an automated script:**
+


How would you feel about crediting yourself here? (Like "Implemented by @Wiguwbe" or something)

I'm okay with that, let us solve these problems first :)

plbrault · 2023-08-19T15:38:11Z

automated_example.py

+
+
+#
+# the main entrypoint to run the scheduler


I'm wondering if this should be a docstring instead

I was actually thinking in changing the example to have it object oriented and make it a skeleton instead.

plbrault · 2023-08-19T15:39:32Z

run-desktop.py

@@ -3,5 +3,6 @@

 subprocess.run([
    'python',
-    'main.py'
+    'main.py',
+    *sys.argv[1:]


Maybe assign this to a variable with a self-explanatory name first, to make it clearer what this argument is

I think it could also make sense to have a custom entrypoint script for the automated run.
This would just display the Game scene and wouldn't even need to handle (much of) pygame/user input.
It would also have the difficulty configuration from CLI.
I'll leave that for last 😄 , in the meantime I'll do as suggested :)

Looking back, this was a quick work-around to pass the extra arguments coming from pipenv run into the script.

As far as I'm aware, we can't assign the first argument to a variable because it may not even exist. And I don't know how sys.args (or bash/exec) handles python's None in arguments (which would be the default).

An option would be to create 2 explanatory lists (with and without file argument), but I think I would prefer to go with a dedicated script/entrypoint.

A dedicated entry point would make sense, maby something like auto.py instead of main. And it could take a difficulty level or custom config as an argument.

That's the idea, to configure the difficulty, even allowing to customize the difficulty from the command line, something like:

# set base difficulty "easy" but set num_cpus to 6 pipenv run auto --easy --num-cpus 6 #...

@Wiguwbe should this argument be removed now?

plbrault · 2023-08-19T15:43:22Z

src/game_objects/io_queue.py

@@ -32,13 +38,13 @@ def _check_if_clicked_on(self, event):
            return self._view.collides(*event.get_property('position'))
        return False

-    def _on_click(self):
+    def on_click(self):


I think I would prefer to make the process_events method public instead of on_click.

plbrault · 2023-08-19T15:52:24Z

src/game_objects/page.py

        self._in_use = value

    def _check_if_clicked_on(self, event):
        if event.type in [GameEventType.MOUSE_LEFT_CLICK, GameEventType.MOUSE_LEFT_DRAG]:
            return self._view.collides(*event.get_property('position'))
        return False

-    def _on_click(self):
+    def on_click(self):


Here I would do like in IoQueue with the process_events method: wrap the line self._page_manager.swap_page(self) into a swap method and call it in _on_click. Then this one could be public instead of _on_click. I would still keep the _on_click method even if it might seem superfluous, as I like having a very clear entrypoint for when a click happens.

plbrault · 2023-08-19T16:05:58Z

src/game_objects/process.py


    def _check_if_clicked_on(self, event):
        if event.type == GameEventType.MOUSE_LEFT_CLICK:
            return self.starvation_level < 6 and self._view.collides(
                *event.get_property('position'))
        return False

-    def _on_click(self):
+    def on_click(self):


Same here, would create an intermediary method and make that one public instead. Maybe toggle, but I'm open to better ideas.

plbrault · 2023-08-19T16:09:44Z

src/lib/event_manager.py

@@ -0,0 +1,110 @@
+# event manager to gather events from game objects and


This should probably be a docstring

plbrault · 2023-08-19T16:12:49Z

src/scenes/game.py

+        # for automation
+        self._script_callback = None
+        if self._script:
+            # pylint: disable=exec-used


Could you move lines 77 to 94 to another method?

plbrault · 2023-08-19T16:15:34Z

src/scenes/game.py

@@ -137,5 +173,17 @@ def update(self, current_time, events):
        if dialog is not None:
            dialog.update(current_time, events)
        else:
+            # first, the script generated events
+            for event in self._get_script_events():


Would also move this block to another method

plbrault · 2023-08-19T16:17:38Z

src/game_objects/io_queue.py

 from lib.game_object import GameObject
 from lib.game_event_type import GameEventType
 from game_objects.views.io_queue_view import IoQueueView


 class IoQueue(GameObject):
+
+    Instance = None


I am not fond of Instances , Pages and Processes being directly in the classes. Could these be held elsewhere?

It makes sense, I believe both ProcessManager and PageManager can hold these dicts

the "on_click" methods return to private, the underlying calls are now extracted into their own methods and made public; PageManager's `_NUM_RAM_COLS` renamed to `_NUM_COLS`; module's docstring added to event_manager; blocks of code inside `scenes/game.py` related to automation are extracted to their own methods; class instances/maps (`IoQueue.Instance`, `Page.Pages`, `Process.Processes`) were made instance variables. PageManager and ProcessManager now function also (slightly) as dictionaries/maps of the pages/processes.

Wiguwbe · 2023-08-19T18:03:19Z

Latest commit should address most of the suggestions, those it doesn't, they are explained :)

Regarding the request for changes, keep them coming :)

wviana · 2023-08-19T19:38:28Z

automated_example.py

+#	'PAGE_NEW',
+#	'PAGE_USE',
+#	'PAGE_SWAP',
+#   'PAGE_FREE',


Indentation

The event_manager was also changed to pass the event type as a string (more useful for the current implementation of the skeleton) With that, there is no need to pass the event types "enum" to the automation script

Wiguwbe · 2023-08-19T21:02:26Z

The example was now renamed to skeleton and now provides a more OO approach

It is used to run the game with an automated script. The script parses the filename and difficulty configuration from the command line. Only events from the script are processed (no clicks at all) The game scene now has a standalone mode option, which removes all buttons. The game over dialog doesn't have buttons either, in standalone mode. The script only launches the standalone Game scene. A small fix on the automated skeleton was also provided.

Wiguwbe · 2023-08-20T11:47:01Z

src/scenes/game.py

@@ -12,10 +14,14 @@


 class Game(Scene):
-    def __init__(self, screen, scenes, config=None):
+    # pylint: disable=too-many-arguments


to support the extra standalone argument, the too-many-arguments error would appear (6/5)

Wiguwbe · 2023-08-20T11:47:15Z

src/game_objects/game_over_dialog.py

@@ -4,28 +4,31 @@


 class GameOverDialog(GameObject):
-
-    def __init__(self, uptime, score, restart_game_fn, main_menu_fn):
+    # pylint: disable=too-many-arguments


to support the extra standalone argument, the too-many-arguments error would appear (6/5)
(same as scene/game.py)

Wiguwbe · 2023-08-20T15:01:03Z

I made the auto entrypoint and modified the Game scene to be able to run "standalone"

plbrault · 2023-08-20T17:47:02Z

run-desktop.py

@@ -3,5 +3,6 @@

 subprocess.run([
    'python',
-    'main.py'
+    'main.py',
+    *sys.argv[1:]


@Wiguwbe should this argument be removed now?

plbrault · 2023-08-21T17:09:02Z

Looks all good to me! Is this ready to be merged?

Wiguwbe · 2023-08-21T18:58:00Z

let me just add the credit :)

Wiguwbe · 2023-08-21T19:00:30Z

Whenever you're ready @plbrault

Wiguwbe added 3 commits August 18, 2023 20:25

update README

29a06e3

remove unused enum and unreachable code

cf06a72

Wiguwbe commented Aug 18, 2023

View reviewed changes

revert unused change

9c150e4

it was previously changed to accomodate events coming from the automation script. as the way those events were handled changed, this change was left behind

plbrault linked an issue Aug 18, 2023 that may be closed by this pull request

[Feature Request] Autoplay #70

Closed

Wiguwbe added 3 commits August 19, 2023 10:30

remove duplicate section

18fa551

merge plbrault/main into Wiguwbe/auto

1f28491

Fix pylint warnings

5b3b4cf

Wiguwbe commented Aug 19, 2023

View reviewed changes

plbrault requested changes Aug 19, 2023

View reviewed changes

Merge branch 'main' into auto

fb206f2

wviana reviewed Aug 19, 2023

View reviewed changes

Provide object oriented automation skeleton

ed881d2

The event_manager was also changed to pass the event type as a string (more useful for the current implementation of the skeleton) With that, there is no need to pass the event types "enum" to the automation script

plbrault and others added 2 commits August 19, 2023 21:07

Merge branch 'main' into auto

b89ee5d

Wiguwbe commented Aug 20, 2023

View reviewed changes

plbrault requested changes Aug 20, 2023

View reviewed changes

plbrault and others added 2 commits August 20, 2023 14:50

Merge branch 'main' into auto

1a3df57

merge branch main into auto

924c599

Wiguwbe added 2 commits August 21, 2023 11:32

remove extra run-desktop arguments

7b9ba9e

pylint on Game scene

4e485e5

plbrault approved these changes Aug 21, 2023

View reviewed changes

update readme for credits and auto options

621e280

plbrault merged commit 8174b7b into plbrault:main Aug 21, 2023

		@@ -0,0 +1,110 @@
		# event manager to gather events from game objects and

Implement automation mechanism #78

Implement automation mechanism #78

Conversation

Wiguwbe commented Aug 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wiguwbe commented Aug 19, 2023

plbrault left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wiguwbe commented Aug 19, 2023

Choose a reason for hiding this comment

Wiguwbe commented Aug 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wiguwbe commented Aug 20, 2023

Choose a reason for hiding this comment

plbrault commented Aug 21, 2023

Wiguwbe commented Aug 21, 2023

Wiguwbe commented Aug 21, 2023