nesbox / TIC-80

TIC-80 is a fantasy computer for making, playing and sharing tiny games.
https://tic80.com
MIT License
4.98k stars 479 forks source link

feature: sspr() function: multi tile sprite operations and palette swapping #71

Closed ambyra closed 7 years ago

ambyra commented 7 years ago

spr() function, but extended to operate #on multiple tiles at once. Useful for larger sprites. Similar to pico 8 function: spr( n, x, y, [w,] [h,] [flip_x,] [flip_y] )

A useful addition would be per-sprite palette swaps, for color cycling effects and animations, like on the real NES. In this case, a palette editor, and palette selector in the sprite editor would be handy.

sspr(id, x, y, [w [h [colorkey [pal_id [scale [flip [rotate] ] ] ] ] ] ])

hashalon commented 7 years ago

why naming it 'sspr' ? what does the extra 's' stands for ?

nesbox commented 7 years ago

It calls sspr in pico, I haven't given name in TIC. Any suggestions?

ambyra commented 7 years ago

The pico8 spr() handles tile regions, sspr() handles pixel regions from the sprite sheet and optionally scales them. The functionality I asked for is in line with pico8's regular spr() function.

AnastasiaDunbar commented 7 years ago

It would be better if the palette swapping itself is a function instead so you can change the colours for the whole screen or only for drawing operations, otherwise maybe drawing the map would require a palette swapping parameter too. I've created an issue here #104. EDIT: Nevermind. I think it's very important to have a sprite function that can take pixel regions.

nesbox commented 7 years ago

added sprite id x y [w=1 h=1] [colorkey=-1] [scale=1] [flip=0] [rotate=0] api to draw composite sprites (don't know what to do with spr api, maybe mark it as deprecated or leave both???)

done on .23

trelemar commented 7 years ago

I'd leave spr for now at least until users can update the code to the new function. That way it doesn't break every demo and game on the website.

nesbox commented 7 years ago

or I can add width/height parameters to the end of spr to avoid api duplication spr id x y [colorkey=-1] [scale=1] [flip=0] [rotate=0] [w=1 h=1] what do you think?

AnastasiaDunbar commented 7 years ago

@nesbox It will confuse users. Especially people who have their old projects using the spr function.

nesbox commented 7 years ago

ok, just renamed sprite to cspr (composite sprite) so, to draw 1x1 sprite call spr id x y [colorkey=-1] [scale=1] [flip=0] [rotate=0] to draw composite sprite call cspr id x y [w=1 h=1] [colorkey=-1] [scale=1] [flip=0] [rotate=0]

thanks

HomineLudens commented 7 years ago

My preference goes to spr with additional parameters appended at the end.. No api duplication and compatibility

nesbox commented 7 years ago

damn, can't choose, I like both variants 😢

AnastasiaDunbar commented 7 years ago

@nesbox Sorry, use the width and height parameters at the end of the spr function. I thought spr function already had width and height parameters.

ambyra commented 7 years ago

Problem is really the function uses too many parameters. There should be a function to grab a tile, another to scale it, another to adjust the mask, etc. Just cause pico8 does it one way, doesn't mean it's the right way.

asprite=spr(123,2,2) .rotate(3) .scale(2) .etc(69)

nesbox commented 7 years ago

@to90zeroblue you're right, but unfortunately we also have compatibility problem

AnastasiaDunbar commented 7 years ago

@to90zeroblue There's also an other way: spr(id,x,y,{"width"=4,"height"=5,"colorkey"=10}) I prefer parameters instead.

trelemar commented 7 years ago

There's a few different solutions to this I think. You could allow named arguments through a table, or have the first parameter be id, w, h if it's a table, else just the id. It would be pretty messy if width and height were the last parameters, they should be after id.

spr({id,w,h} or id,x,y,colorkey,scale,flip,rotate)
ambyra commented 7 years ago

Using names is great too! Still too many variables. Keep Spr(), make sspr() that can only return a region, and implement functions on it flip(), rotate(), mask(), scale(),draw(),palette()

AnastasiaDunbar commented 7 years ago

@to90zeroblue But how would it know when to draw the sprite when you have optional attributes? Just trying to say that sprite isn't an object, if they were objects then you have to manage them. It would look like:

mySprite=spr(0,5,5)
mySprite.scale(3)
function TIC()
    cls()
    mySprite.draw()
end
ambyra commented 7 years ago

Would have to return a sprite region, and you delete it when done. It would be pretty useful, since most sprites are used for many frames.

ambyra commented 7 years ago

https://sourcemaking.com/refactoring/smells/long-parameter-list

ambyra commented 7 years ago

I think that version looks really nice Anastasia :) Easy to read and understand, and short. Will look good on a small tic80 screen.

ambyra commented 7 years ago

But it would by mySprite=SSpr(0,5,5). Leave spr() alone for a while till people switch to new API, mark deprecated.

trelemar commented 7 years ago

I'm not a huge fan of that workflow. For a new user what's the difference between the following?

myspr=spr(16,5,5)
--or
function TIC()
  spr(16,5,5)
  myspr:draw()
end

See a problem? When you define the sprite as an object, you define the width and height. But when you just want to draw a single sprite, your defining the x,y of where to draw on screen. It's confusing.

Sprites are not objects, cspr would simply stitch together multiple 8x8 sprites.

Having methods for the larger sprite objects is unlike anything in TIC's API, and wouldn't be very consistent. I don't know...

AnastasiaDunbar commented 7 years ago

Another "problem" is: if sprites are objects, you don't need a table of positions then. For example, from this:

sprites={{id=5,x=2,y=6},{id=2,x=50,y=70}}
for i=1,#sprites do
    spr(sprites[i].id,sprites[i].x,sprites[i].y)
end

To this:

sprites={spr(5,2,6),spr(2,50,70)}
for i=1,#sprites do
    sprites[i]:draw()
end

@trelemar cspr would be useless if it stitched together multiple 8x8 sprites.

trelemar commented 7 years ago

@AnastasiaDunbar isn't that exactly what @nesbox has it do though? How is that useless? That's the entire topic of this.

AnastasiaDunbar commented 7 years ago

@trelemar It should use pixel regions.

trelemar commented 7 years ago

@AnastasiaDunbar then it shouldn't require an id. And why are it's default width and height 1?

AnastasiaDunbar commented 7 years ago

@trelemar Okay, I wasn't thinking of its name being composite. But I just hope there will be a function that draws a selected pixel region from sprite. It's just that it's easy to recreate as:

function cspr(id,x,y,w,h)
    for c=0,w-1 do
        for r=0,h-1 do
            spr(id+c+(r*16),x+(c*8),y+(r*8))
        end
    end
end
AnastasiaDunbar commented 7 years ago

So I tried to make my own function for pixel-region sprite:

function sget(x,y)
    x=math.floor(x);y=math.floor(y)
    return peek4((0x4000+(x//8+y//8*16)*32)*2+x%8+y%8*8)
end
function sprp(px,py,w,h,x,y)
    for c=0,w-1 do
        for r=0,h-1 do
            pix(x+c,y+r,sget(px+c,py+r))
        end
    end
end

The downside is that it's probably slower than an inbuilt function. Also, don't forget about the rotation and mirror parameters, and that when region is out of bounds it should warp around.