Skip to content

fix: address upstream review comments on TypedData migration #7

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.

Sign up for GitHub

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

Merged
takaokouji merged 1 commit into from
Mar 29, 2026
Merged

fix: address upstream review comments on TypedData migration #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions event.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ DEFINE_DATA_TYPE(SDL_Event, free);

static VALUE Event_new(SDL_Event* ev)
{
SDL_Event* e = ALLOC(SDL_Event);
SDL_Event* e;
VALUE obj = TypedData_Make_Struct(event_type_to_class[ev->type], SDL_Event, &SDL_Event_data_type, e);
*e = *ev;
return TypedData_Wrap_Struct(event_type_to_class[ev->type], &SDL_Event_data_type, e);
return obj;
}

static VALUE Event_s_allocate(VALUE klass)
Expand All @@ -102,10 +103,11 @@ static VALUE Event_s_allocate(VALUE klass)
if (event_type == Qnil)
rb_raise(rb_eArgError, "Cannot allocate %s", rb_class2name(klass));

e = ALLOC(SDL_Event);
memset(e, 0, sizeof(SDL_Event));
e->common.type = NUM2INT(event_type);
return TypedData_Wrap_Struct(klass, &SDL_Event_data_type, e);
{
VALUE obj = TypedData_Make_Struct(klass, SDL_Event, &SDL_Event_data_type, e);
e->common.type = NUM2INT(event_type);
return obj;
}
}

/*
Expand Down
5 changes: 3 additions & 2 deletions gamecontroller.c.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,10 @@ DEFINE_DATA_TYPE(GameController, GameController_free);

static VALUE GameController_new(SDL_GameController* controller)
{
GameController* g = ALLOC(GameController);
GameController* g;
VALUE obj = TypedData_Make_Struct(cGameController, GameController, &GameController_data_type, g);
g->controller = controller;
return TypedData_Wrap_Struct(cGameController, &GameController_data_type, g);
return obj;
}

DEFINE_WRAPPER(SDL_GameController, GameController, controller, cGameController,
Expand Down
5 changes: 3 additions & 2 deletions gl.c.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ DEFINE_WRAPPER(SDL_GLContext, GLContext, context, cGLContext, "SDL2::GL::Context

static VALUE GLContext_new(SDL_GLContext context)
{
GLContext* c = ALLOC(GLContext);
GLContext* c;
VALUE obj = TypedData_Make_Struct(cGLContext, GLContext, &GLContext_data_type, c);
c->context = context;
return TypedData_Wrap_Struct(cGLContext, &GLContext_data_type, c);
return obj;
}

/*
Expand Down
5 changes: 3 additions & 2 deletions joystick.c.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ DEFINE_DATA_TYPE(Joystick, Joystick_free);

static VALUE Joystick_new(SDL_Joystick* joystick)
{
Joystick* j = ALLOC(Joystick);
Joystick* j;
VALUE obj = TypedData_Make_Struct(cJoystick, Joystick, &Joystick_data_type, j);
j->joystick = joystick;
return TypedData_Wrap_Struct(cJoystick, &Joystick_data_type, j);
return obj;
}

DEFINE_WRAPPER(SDL_Joystick, Joystick, joystick, cJoystick, "SDL2::Joystick");
Expand Down
10 changes: 6 additions & 4 deletions mixer.c.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ DEFINE_DATA_TYPE(Chunk, Chunk_free);

static VALUE Chunk_new(Mix_Chunk* chunk)
{
Chunk* c = ALLOC(Chunk);
Chunk* c;
VALUE obj = TypedData_Make_Struct(cChunk, Chunk, &Chunk_data_type, c);
c->chunk = chunk;
return TypedData_Wrap_Struct(cChunk, &Chunk_data_type, c);
return obj;
}

DEFINE_WRAPPER(Mix_Chunk, Chunk, chunk, cChunk, "SDL2::Mixer::Chunk");
Expand All @@ -54,9 +55,10 @@ DEFINE_DATA_TYPE(Music, Music_free);

static VALUE Music_new(Mix_Music* music)
{
Music* c = ALLOC(Music);
Music* c;
VALUE obj = TypedData_Make_Struct(cMusic, Music, &Music_data_type, c);
c->music = music;
return TypedData_Wrap_Struct(cMusic, &Music_data_type, c);
return obj;
}

DEFINE_WRAPPER(Mix_Music, Music, music, cMusic, "SDL2::Mixer::Music");
Expand Down
4 changes: 2 additions & 2 deletions rubysdl2_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ void rubysdl2_init_gamecontorller(void);
*/
#define DEFINE_DATA_TYPE(struct_name, free_func) \
static const rb_data_type_t struct_name##_data_type = { \
#struct_name, \
"ruby-sdl2/" #struct_name, \
{ NULL, (void (*)(void*))(free_func), NULL }, \
NULL, NULL, 0 \
NULL, NULL, RUBY_TYPED_FREE_IMMEDIATELY \
};

#define DEFINE_GETTER(scope, ctype, var_class, classname) \
Expand Down
5 changes: 3 additions & 2 deletions ttf.c.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,10 @@ DEFINE_DATA_TYPE(TTF, TTF_free);

static VALUE TTF_new(TTF_Font* font)
{
TTF* f = ALLOC(TTF);
TTF* f;
VALUE obj = TypedData_Make_Struct(cTTF, TTF, &TTF_data_type, f);
f->font = font;
return TypedData_Wrap_Struct(cTTF, &TTF_data_type, f);
return obj;
}

DEFINE_WRAPPER(TTF_Font, TTF, font, cTTF, "SDL2::TTF");
Expand Down
48 changes: 24 additions & 24 deletions video.c.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,12 @@ static void Window_free(Window* w)

static VALUE Window_new(SDL_Window* window)
{
Window* w = ALLOC(Window);
Window* w;
VALUE obj = TypedData_Make_Struct(cWindow, Window, &Window_data_type, w);
w->window = window;
w->num_renderers = 0;
w->max_renderers = 4;
w->renderers = ALLOC_N(struct Renderer*, 4);
return TypedData_Wrap_Struct(cWindow, &Window_data_type, w);
return obj;
}

DEFINE_GETTER(static, Window, cWindow, "SDL2::Window");
Expand All @@ -124,10 +124,9 @@ static VALUE Display_new(int index)

static VALUE DisplayMode_s_allocate(VALUE klass)
{
SDL_DisplayMode* mode = ALLOC(SDL_DisplayMode);
mode->format = mode->w = mode->h = mode->refresh_rate = 0;
mode->driverdata = NULL;
return TypedData_Wrap_Struct(klass, &SDL_DisplayMode_data_type, mode);
SDL_DisplayMode* mode;
VALUE obj = TypedData_Make_Struct(klass, SDL_DisplayMode, &SDL_DisplayMode_data_type, mode);
return obj;
}

static VALUE DisplayMode_new(SDL_DisplayMode* mode)
Expand Down Expand Up @@ -179,14 +178,14 @@ static void Window_attach_renderer(Window* w, Renderer* r)

static VALUE Renderer_new(SDL_Renderer* renderer, Window* w)
{
Renderer* r = ALLOC(Renderer);
Renderer* r;
VALUE obj = TypedData_Make_Struct(cRenderer, Renderer, &Renderer_data_type, r);
r->renderer = renderer;
r->num_textures = 0;
r->max_textures = 16;
r->textures = ALLOC_N(Texture*, 16);
r->refcount = 1;
Window_attach_renderer(w, r);
return TypedData_Wrap_Struct(cRenderer, &Renderer_data_type, r);
return obj;
}

DEFINE_WRAPPER(SDL_Renderer, Renderer, renderer, cRenderer, "SDL2::Renderer");
Expand Down Expand Up @@ -221,11 +220,12 @@ static void Renderer_attach_texture(Renderer* r, Texture* t)

static VALUE Texture_new(SDL_Texture* texture, Renderer* r)
{
Texture* t = ALLOC(Texture);
Texture* t;
VALUE obj = TypedData_Make_Struct(cTexture, Texture, &Texture_data_type, t);
t->texture = texture;
t->refcount = 1;
Renderer_attach_texture(r, t);
return TypedData_Wrap_Struct(cTexture, &Texture_data_type, t);
return obj;
}

DEFINE_WRAPPER(SDL_Texture, Texture, texture, cTexture, "SDL2::Texture");
Expand All @@ -243,10 +243,10 @@ static void Surface_free(Surface* s)

VALUE Surface_new(SDL_Surface* surface)
{
Surface* s = ALLOC(Surface);
Surface* s;
VALUE obj = TypedData_Make_Struct(cSurface, Surface, &Surface_data_type, s);
s->surface = surface;
s->need_to_free_pixels = 0;
return TypedData_Wrap_Struct(cSurface, &Surface_data_type, s);
return obj;
}

DEFINE_WRAPPER(SDL_Surface, Surface, surface, cSurface, "SDL2::Surface");
Expand Down Expand Up @@ -2106,6 +2106,7 @@ static VALUE Surface_s_from_string(int argc, VALUE* argv, VALUE self)
SDL_Surface* surface;
void* pixels;
Surface* s;
VALUE obj;

rb_scan_args(argc, argv, "45", &string, &width, &height, &depth,
&pitch, &Rmask, &Gmask, &Bmask, &Amask);
Expand All @@ -2132,10 +2133,10 @@ static VALUE Surface_s_from_string(int argc, VALUE* argv, VALUE self)

RB_GC_GUARD(string);

s = ALLOC(Surface);
obj = TypedData_Make_Struct(cSurface, Surface, &Surface_data_type, s);
s->surface = surface;
s->need_to_free_pixels = 1;
return TypedData_Wrap_Struct(cSurface, &Surface_data_type, s);
return obj;
}

/*
Expand Down Expand Up @@ -2535,10 +2536,9 @@ static VALUE Surface_s_new(int argc, VALUE* argv, VALUE self)
*/
static VALUE Rect_s_allocate(VALUE klass)
{
SDL_Rect* rect = ALLOC(SDL_Rect);
rect->x = rect->y = rect->w = rect->h = 0;

return TypedData_Wrap_Struct(cRect, &SDL_Rect_data_type, rect);
SDL_Rect* rect;
VALUE obj = TypedData_Make_Struct(klass, SDL_Rect, &SDL_Rect_data_type, rect);
return obj;
}

/*
Expand Down Expand Up @@ -2640,9 +2640,9 @@ static VALUE Rect_union(VALUE self, VALUE other)
*/
static VALUE Point_s_allocate(VALUE klass)
{
SDL_Point* point = ALLOC(SDL_Point);
memset(point, 0, sizeof(SDL_Point));
return TypedData_Wrap_Struct(klass, &SDL_Point_data_type, point);
SDL_Point* point;
VALUE obj = TypedData_Make_Struct(klass, SDL_Point, &SDL_Point_data_type, point);
return obj;
}

/*
Expand Down