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

Only show PewPew games in the menu #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cwalther
Copy link
Contributor

On the ESP8266, in addition to any PewPew games, I have files boot.py and webrepl_cfg.py in the filesystem root. There may be other libraries too. These all show up in the PewPew menu (menu.py), even though launching them as games makes no sense.

The attached commit filters the directory listing to files that are recognized as PewPew games by checking whether one of their first two lines is import pew. Specifying that it must be the first line would make the code even simpler, but this is currently not satisfied by tetris.py.

What do you think about this?

This excludes e.g. boot.py and webrepl_cfg.py, which are usually present on ESP8266 boards, but not useful to launch from the menu.
@deshipu
Copy link
Collaborator

deshipu commented Jul 19, 2018

To be honest, I would prefer to not look into the files. How about just having a games directory where you can put all your games?

I'm also thinking about not having the games run their code on import, but instead have something like a run() function — that would improve memory management with Python 3.0, and would also make it easier to reuse code.

@cwalther
Copy link
Contributor Author

A run() function sounds like a good idea – using “import” as “run script” always feels like a bit of a misuse. (Though you have to take care to clear it from sys.modules on updates either way.)

The disadvantage of a “games” directory is that the WebREPL HTML client only allows putting files in the root (or I haven’t figured out how to use subdirectories yet – webrepl_cli.py can do it). I don’t have a better idea though if the goal is to avoid looking into the files, short of a naming convention, which I’m not super-enthusiastic about. I guess the HTML client could be extended if needed.

@deshipu
Copy link
Collaborator

deshipu commented Jul 19, 2018

I wasn't aware about that limitation of the webrepl client. Let's consider our options:

  • a blacklist of files to ignore (boot, main) — I think that's how I do it now, we just need to extend it;
  • a dedicated directory;
  • a naming convention:
    • prefix
    • suffix
    • extension (since we have to compile custom firmware anyways, we can add one)
  • scanning the content;
  • configuration with an explicit list;
  • additional file with game metadata and its entry point per game.

@cwalther
Copy link
Contributor Author

I was wrong about the WebREPL – it sends files using a relative destination path, so they end up in the current directory, and you can os.chdir() around to put them anywhere you like. I’m not sure if that’s a feature or a bug (it might clash with a running program having its own requirements on the CWD, and you can’t change it while a program is running), but it’s sufficient to retract my concern about a “games” directory. (Of course I only discovered that after I had implemented a directory entry box, when testing that.)

Of the options you list, the dedicated directory sounds like the simplest and least restricting or inconvenient solution, apart from looking at the file content.

I’m curious, what is it that you don’t like about looking into the files? I don’t insist on using import pew as the magic marker, that just seemed like an elegant heuristic that would cover most existing setups unchanged. It occurs to me it wouldn’t work with byte-compiled games, but then these are already excluded by the current .py name filter, and you could still have a small launcher .py that imports the bulk of the code from a compiled module.

Why do we have to compile a custom firmware? I certainly haven’t, so far. Is that a difference between ESP8266 (which I am using) and SAMD platforms?

@deshipu
Copy link
Collaborator

deshipu commented Jul 30, 2018

Sorry for the delay, I was travelling and couldn't answer.

You are right, you don't have to compile your own firmware with the current generation of PewPew boards — I confused it with µGame, which needs a small part written in C. So the extension option is not viable.

I'm not sure what is my problem with looking at the contents — it just that opening and reading every single file seems wrong. We could probably look at a hashbang at the beginning of the file, or something similar. It still doesn't feel right, but that's how all the desktop file managers do that...

I think that a separate metadata file is the best way to go with respect to flexibility and future development. You can put a user-friendly game name in it, some additional information like author and copyright, maybe even an icon, and the entry point for starting the game (what file to import and what function to call). The down side is that then you would need two files for creating a new game, but I'm not sure it's that bad?

@cwalther
Copy link
Contributor Author

It seemed like less of a hassle for developers and users to have the metadata and the Python code in the same file, but you are right, it has disadvantages too – can’t filter by name extension, doesn’t work with compiled files.

I don’t have a strong opinion on this, go with what you think works best. Most importantly, choose what fits best into your vision of the PewPew/µGame ecosystem. You obviously have the better overview there than I do.

@yeyeto2788
Copy link

@cwalther we might need to think about another way to do such thing since the change you're proposing in this PR might have some drawback, think for a moment that someone has a comment on the import pewpew statement and the method would always return False as in the code below.

>>> a = "import pewpew  # importing pewpew module.      "
>>> a.strip()
'import pewpew  # importing pewpew module.'
>>> a.strip() == "import pewpew"
False

Maybe it is safer to use regex when verifying if the "game" has that import statement.

Regards.

@cwalther
Copy link
Contributor Author

I wouldn’t consider it too much to ask of people to have the magic line be exactly import pew without comments, additional imports or anything – but then the point seems moot anyway as we seem to be moving away from that solution.

@cwalther
Copy link
Contributor Author

cwalther commented Sep 2, 2019

We should also think about excluding dot-files from the menu. When saving a file foo.py using GUI tools on macOS, it also creates a file ._foo.py to store metadata that the FAT filesystem can’t handle. Unfortunately there is no way to stop it from doing that. These then show up in the menu and confuse users, making people think they have lost their game, and selecting them crashes the menu. (We might even delete them, as they are useless in the vast majority of cases and use up space that we’d rather use for code, but I’m uneasy about deleting stuff without asking.) Myself, I’ve grown accustomed to using cp -X (exclude extended attributes) instead of the Finder to copy files to the device, but that is obviously not a solution for beginners.

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.

None yet

3 participants