Some functions should be rewritten into methods

[malashin]

For example this three can be methods for *Window, *Surface and *GameController:

// CreateRenderer returns a new 2D rendering context for a window.
// (https://wiki.libsdl.org/SDL_CreateRenderer)
func CreateRenderer(window *Window, index int, flags uint32) (*Renderer, error) {...}

// CreateSoftwareRenderer returns a new 2D software rendering context for a surface.
// (https://wiki.libsdl.org/SDL_CreateSoftwareRenderer)
func CreateSoftwareRenderer(surface *Surface) (*Renderer, error) {...}

// GameControllerMapping returns the current mapping of a Game Controller.
//  (https://wiki.libsdl.org/SDL_GameControllerMapping)
func GameControllerMapping(ctrl *GameController) string {...}

You can find more with this regexp expression:

^func [A-Z][a-zA-Z_0-9]+\([a-zA-Z_0-9]+ \*[a-zA-Z_0-9]

[dodobyte]

I speak only for CreateRenderer, please don't do it. Just because renderer needs a window to draw doesn't make it window's method. It's a different animal.

A window should have methods only to manipulate that window. Any objects should have methods only to manipulate itself. Some external objects may want to use your window object, in this case a renderer, so be it. But they are not your concern. You cannot write a new method for your window every time somebody uses it to init their object.

For instance, bytes.NewBuffer is not a method of []byte.

Just look at the code, i think it looks weird.

renderer := window.CreateRenderer()

renderer.Clear()
renderer.Copy()
renderer.DrawRect()
renderer.Present()

renderer.Destroy()

Please remember i didn't check any other function, i merely talk about CreateRenderer. It might be a great proposal overall. Thanks for the great job.

[malashin]

@dodobyte if this is the case for object creation, then we have some rogue methods that already do this as methods.

// CreateTexture returns a new texture for a rendering context.
// (https://wiki.libsdl.org/SDL_CreateTexture)
func (renderer *Renderer) CreateTexture(format uint32, access int, w, h int32) (*Texture, error) {...}

// CreateTextureFromSurface returns a new texture from an existing surface.
// (https://wiki.libsdl.org/SDL_CreateTextureFromSurface)
func (renderer *Renderer) CreateTextureFromSurface(surface *Surface) (*Texture, error) {...}

This one is weird, not a simple getter. Though I'd rather leave is as it is now.

// GetSurface returns the SDL surface associated with the window.
// (https://wiki.libsdl.org/SDL_GetWindowSurface)
func (window *Window) GetSurface() (*Surface, error) {...}

A new surface will be created with the optimal format for the window, if necessary. This surface will be freed when the window is destroyed. Do not free this surface. This surface will be invalidated if the window is resized. After resizing a window this function must be called again to return a valid surface.

We need to decide what to do with those.

---

This one definitely needs to be rewritten, since its a simple getter:

// GameControllerMapping returns the current mapping of a Game Controller.
//  (https://wiki.libsdl.org/SDL_GameControllerMapping)
func GameControllerMapping(ctrl *GameController) string {...}

[dodobyte]

@malashin, i see. Consistency is more important than anything else. SDL doesn't imply what should be a method of what. But there are some hints in documentation, namely categories.

I believe the original creator of this library also took categories into consideration. File names show that.

API's in CategoryRender should be methods of *Renderer. The creator function is CreateRenderer, so it must be a top level function, not a method.

CreateTexture and CreateTextureFromSurface functions are in the Render category, so the current configuration is correct for them.

One may argue that CreateTextureFromSurface must be a method of *Surface. In this case we must ask two questions.

• What is the first parameter? Is it *Renderer or *Surface. The first parameter must be the receiver. For this case *Renderer.
• What is the function's category? For this case CategoryRender.

So it should be obvious.

Window functions such as CreateWindow are found in CategoryVideo. Again, the creator function CreateWindow should be a top level.

---

Finally, CreateRenderer. Should it be a method of *Window or a top level function. Let's ask the questions.

• Is function's first parameter a *Window? Check.
• Is function in the CategoryVideo? No.

So it's not a method of *Window.

For CreateRenderer it's not hard to decide because it's the creator function of a whole category. But for less obvious ones this two questions can help you to decide.

Thanks.

[veeableful]

I fixed the GameControllerMapping() first as it definitely needs to be made a method.

I'm not really sure about what to do about the rest. Both of you have some valid points. I think we should wait until there's a strong case for either of the options before we make further changes. If it is already working okay for everyone then we don't need to make any changes. I was thinking of providing both options (method and function) but I guess people wouldn't like that in the long run :P

[macroblock]

First of all, the reason which @dodobyte mentioned (function's category) is not a good point to consider it seriously. Renderer has its own category just because it is big enough to have it. Further I will try to give a reason why @malashin is right:

• Renderer is actually part of Window struct (inside SDL library).
• Window is not "just an argument". Renderer can't have normal functionality after destroying the Window from which it was created.
• One Renderer can have output only to one Window. Window doesn't let you create more than one Renderer.
• Renderer can't change/rebind/reattach/etc its Window. When Renderer loses a Window it loses functionality.
• If we set Renderer's target (SetRenderTarget) to a texture. It doesn't mean that we detached it from Window. It means that we just redirected a Renderer's output to a texture. If we destroy a Renderer's Window then Renderer will lose functionality.
• If we set Renderer's target to NULL then Renderer restores an output to a Window from which it was created.

I think it transparently shows up 'HAS-A' relations. I.e:

• Renderer has Textures so CreateTexture is a part of Renderer.
• Window has Renderer so by consistency reasons CreateRenderer should be a part of Window.