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

feat: add 4BPP/16colors sprites handling #250

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

youenchene
Copy link
Contributor

@youenchene youenchene commented Feb 5, 2025

Description

Advanced Sprites Manager is an overlay to the sprite manager.

It allows you to directly use:

  • simple 4 colors & 16px wide sprites (same as the Sprites Manager).
  • 4 colors & 32px wide sprites.
  • 16 colors & 16px wide sprites.
  • 16 colors & 32px wide sprites.

Motivation and Context

16 colors sprite needed a lot of repeatitive same code. (Vairn example) https://github.com/Vairn/SmitACE/blob/main/src/misc/mouse_pointer.c

I was hoping for a more direct way as in Blitz Basic 2 or like SGDK for megadrive/genesis.

How Has This Been Tested?

Here : https://github.com/youenchene/amiga-test-ace-sprites/tree/16colors-sprite

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@youenchene
Copy link
Contributor Author

My next contribution should be an animatedSprite manager to handle animation in a optimized way especially in 4BPP.

Copy link
Member

@tehKaiN tehKaiN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but we can't have this in current state, because of performance reasons. Such extra logic could and most probably should be done on game's side, not on ACE's side, in a game-specific way.

When you have such changes battle-tested on your game code (preferably not turn-based but 50fps one) and universal enough, we can get back to it and see if it's universal enough to merge it.

@youenchene
Copy link
Contributor Author

I agree on the performance side. That s why i put in the documentation and on the fonction comments that for 4bpp it's not suitable for animation.

I'be back in new version.

A advancedSprite manager to handle:

  • 4bpp sprites with prepared bitmap set for animation.
  • 32px wide sprites (group of sprites).
  • multiplexed sprites

(Not in one pull request, this is a roadmap following my requirements to make progress on my Homebrew game).

@youenchene youenchene reopened this Feb 15, 2025
@youenchene youenchene requested a review from tehKaiN February 15, 2025 10:16
@youenchene
Copy link
Contributor Author

@tehKaiN my feature is back as an advanced Sprite Manager :

Feature :

  • Frames/animation
  • 32px sprites
  • 16 colors sprites

@youenchene
Copy link
Contributor Author

youenchene commented Feb 15, 2025

@tehKaiN
Copy link
Member

tehKaiN commented Feb 15, 2025

I've glanced through the code and it indeed looks better allocation-wise. I'll look at it over the next week.

One nitpick though - "advanced" is a bit generic naming, since if someone creates an improved version based on it, how it would be called? "Advancer"? "Uber"? ;) Still, that might be the best naming that's out there since it improved on multiple things at once. I have no other proposal as for now, just food for thought.

Have you tried building a game around it, apart from some test code? My bob system was incorporated to ACE only after I've went with it through 3 of them, and that allowed me to refine it for different use cases and add some optimizations for stuff not needed in some of them. It would be best if it goes at least through one more or less complete game.

@youenchene
Copy link
Contributor Author

No problem if you find improvement in the mem allocation.

For the name I have no inspiration.

In fact I am building a game, while I am doing this contribs.

The risk is minimize as it's an "overlay" of the sprites manager. It can be marked as alpha/beta/incubing, whatever.

For example, I'll add the sprite multiplexer when I'll add the enemies.

@youenchene
Copy link
Contributor Author

Indeed, integrating into my game and saw 2 fixes and an optimization.

@tehKaiN
Copy link
Member

tehKaiN commented Mar 1, 2025

Sorry for the delay, but I'm afraid I still don't have time to review this thing.

I remember about this PR on a daily basis, but I'm in a frenzy of finishing my game and the deadline is quite near. I'll try to review it asap, but please don't wait for it - just keep using it in your game, perhaps polishing it in the process, and I'll get back to you as soon as I can.

@youenchene
Copy link
Contributor Author

No worries.

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.

2 participants