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

User should be warned when $position(arg[0], arg[1]) returns a value outside of the screen #11

Closed
cirillom opened this issue Sep 22, 2022 · 7 comments · Fixed by #15
Closed
Assignees
Labels
enhancement New feature or request

Comments

@cirillom
Copy link
Member

When a user tries to print a string with 40 or more characters from the middle of the screen using:

  loadn r1, $position(eval((40-int(defs["var_len"]))//2),0) 

defs["var_len"] will be greater than 40, when this happens the first argument of $position(arg[0], arg[1]) is a negative value, and the screen can only print from 0 to 1199, so the user should be warned when $position(arg[0], arg[1]) returns a value outside this range.

Originally posted by @cirillom in #10 (comment)

@cirillom cirillom added the documentation Improvements or additions to documentation label Sep 22, 2022
@lucasgpulcinelli lucasgpulcinelli added enhancement New feature or request and removed documentation Improvements or additions to documentation labels Sep 22, 2022
@lucasgpulcinelli
Copy link
Collaborator

That's a complicated topic.

Because position() is written as any other macro, and as such is just evaluated as a simple python expression, it cannot generate more complex code (for instance something that would print a warning or raise an exeption). The only way I see to make this warning is to rewrite the macro creation function such that it executes exec (instead of eval) to create functions with the name of the macro. But at this point, why not just add another python file to the assembly source and import it as usual for the preprocessing?

My personal opinion is that the preprocessor should not be complex at this point, because it kills the aspect of creating small and quick utilities to make code more readable (for instance, anyone can see the code #define position() arg[0]+arg[1]*screen_width and understand it, as opposed to a magic number loading in a register).

Of course, the macro you showed is not the most readable (and I am the one who wrote it), but this is more of a demonstration than anything, because, as you know, it does not work in runtime with different strings.

What do you think?

@lucasgpulcinelli lucasgpulcinelli pinned this issue Sep 22, 2022
@cirillom
Copy link
Member Author

@lucasgpulcinelli I was thinking about something rather simple, almost hard coded.

$position() will always be used to calculate a screen position with arg[0] + arg[1] * screen_width, after this calculation is done by the python script it's printed into the final .asm file.

If we could stop the preprocessing with an exit error simply warning the user: "You're trying to print to an invalid screen position" I think this isn't a very complex feature to add to a small and quick utility, however I'm not completely sure how the macros are built, is it simple to do a verification like:

if (macro == position)
 if(printedValue < 0)
  ERRORWARNING

If it's more complex than this I completely agree with you, and it'll become unnecessarily complex.

@lucasgpulcinelli
Copy link
Collaborator

Well, this solves the negative value problem (I agree that the program should error out if a negative value is to be substituted, for any macro). However I would personally not like a hardcoded solution for the upper limit of 40*30 characters.

@cirillom
Copy link
Member Author

Can't we just use

if (printedValue > sw*sh)
  ERRORWARNING

?

@lucasgpulcinelli
Copy link
Collaborator

We can, of course, but what I'm saying is that I don't like the idea of having a hardcoded solution for this upper limit of this one macro only

@cirillom
Copy link
Member Author

After reading the documentation I understood better the ideia of this macros, and I agree with you. The only "solution" I could think for this is implementing ternaries for macros, but then we would have a compiler. I reckon we can close the issue and mark it as wontfix

@lucasgpulcinelli
Copy link
Collaborator

The negative value problem could be solved easily still, I'll work on that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants