search

kyren / piccolo

An experimental stackless Lua VM implemented in pure Rust
Creative Commons Zero v1.0 Universal
1.62k stars 59 forks source link

Arithmetic metamethods #57

Closed ZayadNimrod closed 2 months ago

ZayadNimrod commented 4 months ago

I found myself needing to override the addition operator (2d position+offset arithmetic). I've only implemented overriding __add and __sub so far; if this is how you'd like the other arithmetic metamethods implemented, I'll implement those too.

ZayadNimrod commented 4 months ago

This should address the suggestions re: error messages. That said, this requires compile-time construction of those strings (using format! means constructing a string every time an error is raised, and to make those the &str that TypeError takes would require leaking the constructed String), so I've brought in a macro crate to handle that. Not sure if the resulting code is particularly nice to work with, and the extra dependency is probably undesirable.

ZayadNimrod commented 4 months ago

Not quite comfortable with erasing error sources like this so I'm keeping the commit in until a last-minute history squash.

Jengamon commented 4 months ago

Not quite comfortable with erasing error sources like this so I'm keeping the commit in until a last-minute history squash.

Note that I'm not telling you to do X or Y, so if you're not comfortable with erasing error sources, don't! Find a way to get your nice errors. Just be aware that I am not operating under any official capacity. My suggestions don't really have binding force, or any force at all. You should do you. I was just questioning the necessity of bringing in an entire other crate for the const evaluation of formatted strings when we could just not, and figure out a way to generate w/e error we want with w/e data we want.

ZayadNimrod commented 4 months ago

Unsure if bc144f0d412db2da66c31f05936216a16504afa7 should make it in, since it should result in extra lookups to the metatable. But it does make the code more compact and less duplicated. Either way this will need a bunch of history surgery once the code is settled.

ZayadNimrod commented 3 months ago

Sorry to ping again, but it's been a month, any thoughts on this before I add the remainder of the arithmetic methods?

kyren commented 2 months ago

I'm very sorry for dropping the ball on this PR, I should have been more responsive. 😔

This has been implemented in #74 though, so we have a very nearly complete set of implemented metamethods now!

Sorry you had to do all this work for a PR that was never merged, but I still very much appreciate the effort you spent, thank you. ❤️