Closed SamsTheNerd closed 2 years ago
DPS2004
commented
2 years ago Builds and runs fine for me, but according to the docs, pal(p) can be a number from 0 to 2, with 2 resetting the fillp palette. It looks like your code doesn't handle that situation, but should be a quick fix I think?
jtothebell
commented
2 years ago Haven't had a chance to run it yet, but it looks fine at first glance. I can add a few unit tests later as well. As far as the 2 parameter value goes, that is for the secondary palette discussed here https://www.lexaloffle.com/bbs/?tid=38565 which FAKE-08 doesn't support yet, so for now I think this should be fine once it gets some tests added. Thanks @SamsTheNerd!
jtothebell
commented
2 years ago It turns out there was one bug in getting the argument from the lua stack. The first argument is at index 1 instead of index 0, so I changed you call from p = lua_tonumber(L, 0); to p = lua_tonumber(L, 1);. There also was a bug with the return value from that configuration of the pal call, and one of the others. Both should be fixed now, and I added a couple unit tests around the new method on the Graphics class as well as an end to end test that verifies the functionality for a few of those pal calls including their return values.
Hello, I noticed that fake08 doesn't seem to support the pal(p) function that resets only a specific palette (documented here).
Given that I'm not very familiar with the details of this code, I don't have the systems set up to test this easily, and it's a relatively small addition, I wasn't sure if it'd have been better to have just posted an issue for this, but I figure it's better to share something than nothing. (I'm especially unsure of how the lua_..(L,) functions work and was mostly just copying from surrounding functions, also not sure if anything else needed to be added to support the function elsewhere)