feat: Ruby 3.4 support — migrate from Data_Wrap_Struct to TypedData#30
Conversation
Replace all deprecated Data_Wrap_Struct/Data_Get_Struct with TypedData_Wrap_Struct/TypedData_Get_Struct across all source files. Add DEFINE_DATA_TYPE helper macro to rubysdl2_internal.h for easy rb_data_type_t definition. Eliminates all deprecation warnings on Ruby 3.4.x while maintaining backward compatibility with Ruby 3.3.x. Files changed: - rubysdl2_internal.h: DEFINE_DATA_TYPE macro, DEFINE_GETTER uses TypedData - video.c.m4: Window, Renderer, Texture, Surface, DisplayMode, Rect, Point - event.c: SDL_Event (EVENT_READER/EVENT_WRITER macros + all event types) - mixer.c.m4: Chunk, Music - ttf.c.m4: TTF - joystick.c.m4: Joystick - gamecontroller.c.m4: GameController - gl.c.m4: GLContext Closes #1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ohai
left a comment
There was a problem hiding this comment.
Thank you for your PR.
It would be better to consider the following points.
| return Data_Make_Struct(klass, SDL_Point, 0, free, point); | ||
| SDL_Point* point = ALLOC(SDL_Point); | ||
| memset(point, 0, sizeof(SDL_Point)); | ||
| return TypedData_Wrap_Struct(klass, &SDL_Point_data_type, point); |
There was a problem hiding this comment.
TypedData_Make_Struct would be better.
There was a problem hiding this comment.
Thank you! Replaced ALLOC + TypedData_Wrap_Struct with TypedData_Make_Struct across all files (video.c.m4, event.c, mixer.c.m4, ttf.c.m4, joystick.c.m4, gamecontroller.c.m4, gl.c.m4). Updated in b9e2a39.
| */ | ||
| #define DEFINE_DATA_TYPE(struct_name, free_func) \ | ||
| static const rb_data_type_t struct_name##_data_type = { \ | ||
| #struct_name, \ |
There was a problem hiding this comment.
It would be better to have a suitable prefix like "ruby-sdl2/".
There was a problem hiding this comment.
Thank you! Added "ruby-sdl2/" prefix. The type name is now "ruby-sdl2/" #struct_name (e.g., "ruby-sdl2/Window"). Updated in b9e2a39.
| static const rb_data_type_t struct_name##_data_type = { \ | ||
| #struct_name, \ | ||
| { NULL, (void (*)(void*))(free_func), NULL }, \ | ||
| NULL, NULL, 0 \ |
There was a problem hiding this comment.
Will RUBY_TYPED_FREE_IMMEDIATELY be better?
There was a problem hiding this comment.
Thank you! Changed from 0 to RUBY_TYPED_FREE_IMMEDIATELY. Updated in b9e2a39.
- Add "ruby-sdl2/" prefix to type names in DEFINE_DATA_TYPE macro - Use RUBY_TYPED_FREE_IMMEDIATELY flag instead of 0 - Replace ALLOC + TypedData_Wrap_Struct with TypedData_Make_Struct Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ohai
left a comment
There was a problem hiding this comment.
The code seems a bit loose to me.
|
|
||
| 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); |
There was a problem hiding this comment.
Assigning to obj seems redundant. return TypedData_Make_Struct(...) looks sufficient.
| 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); |
| Renderer* r; | ||
| VALUE obj = TypedData_Make_Struct(cRenderer, Renderer, &Renderer_data_type, r); | ||
| r->renderer = renderer; | ||
| r->num_textures = 0; |
| Window* w; | ||
| VALUE obj = TypedData_Make_Struct(cWindow, Window, &Window_data_type, w); | ||
| w->window = window; | ||
| w->num_renderers = 0; |
There was a problem hiding this comment.
We can remove the line since TypedData_Make_Struct clears the allocated memory to zeros. However, removing this line will make the code unclear since other parts are explicitly initialized.
I think that this line should be kept.
Return TypedData_Make_Struct directly instead of assigning to a temporary variable in DisplayMode_s_allocate, Rect_s_allocate, and Point_s_allocate where obj is not used before return. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TypedData_Make_Struct zero-clears allocated memory, but explicit initialization of struct members (= 0, = NULL) serves as documentation of what members exist and their expected initial values. Restore removed initializations in Window_new, DisplayMode_s_allocate, Renderer_new, Surface_new, and Rect_s_allocate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
「since other parts are explicitly initialized」と言っているので 0 クリアだけで十分なのはそのままで良かったのですが。まあこれでOKです。 |
|
Thank you for your PR! |
Summary
Migrate all
Data_Wrap_Struct/Data_Get_StructtoTypedData_Wrap_Struct/TypedData_Get_Structfor Ruby 3.4 compatibility.Ruby 3.4 deprecates the old
Data_Wrap_Struct/Data_Get_StructAPI in favor ofTypedData. This PR eliminates all deprecation warnings (previously 90+) while maintaining backward compatibility with Ruby 3.3.x.Approach
A new
DEFINE_DATA_TYPEhelper macro inrubysdl2_internal.hmakes migration straightforward:The existing
DEFINE_GETTERmacro is updated to useTypedData_Get_Struct, and allData_Wrap_Structcalls are replaced withTypedData_Wrap_Struct.Files changed
rubysdl2_internal.h:DEFINE_DATA_TYPEmacro,DEFINE_GETTERusesTypedData_Get_Structvideo.c.m4: Window, Renderer, Texture, Surface, DisplayMode, Rect, Point, FIELD_ACCESSOR macroevent.c: SDL_Event, EVENT_READER/EVENT_WRITER macros, all event type accessorsmixer.c.m4: Chunk, Musicttf.c.m4: TTFjoystick.c.m4: Joystickgamecontroller.c.m4: GameControllergl.c.m4: GLContextTesting