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
How about adding Reset and Printf methods on *text.Text? #88
Comments
I would be happy to send a PR for this if you think it is a good idea to begin with @faiface |
I'm a fan of the And yeah, not sure if Taking my comments into account, I'll be happy to accept a PR :) |
The |
Not much code duplication, but an API duplication. Two ways to do one thing. One thing I really love about Go is that it's avoiding this "there are many ways to solve each problem" thing. I think that's great. It removes mental friction and let's you focus on the problem at hand instead of figuring out which one of those many solutions is the best at any given moment. Furthermore, |
Here is some code that I'd like to improve after doing these changes: https://gist.github.com/peterhellberg/6b7915e9c51c44b8d7e21112cbdf8d0f#file-pixel-pong-go-L146-L148 Eg. I'd love a way to do |
I get that. But, the difference between Imagine we added txt.Printf("%d %d", 1, 2)
txt.Println("Hello, world!")
txt.Print("Yay")
fmt.Fprintz(txt, ...) Inconsistent, you see? In order to make it consistent again, we'd have to add the Of course, it's highly improbable that the |
Ok. I'll listen to what you want, even though I don't agree with the premise that adding support for writing a formatted string to this package would lead to unchecked duplication/feature creep. The (We already have convenience methods like |
Should I remove the unused |
Yeah, no problem. Btw, it just hit me that |
How about |
|
Is there a use case for |
Yeah, when you want to show some text parts by parts. Say word by word, one word at a time. Although that's much less used than dot := txt.Dot
txt.Clear()
txt.Dot = dot This would be a breaking change. But, I'll probably be fine with this change if you also updated all Pixel examples accordingly. Oh, and the tutorial would need to be updated too. If you wanted to do that too, you're welcome, if not, I can do the tutorial. |
I'd prefer not to add a new method if we can do the breaking change for I'll update the examples. I just got the idea that // Clear removes all written text from the Text.
func (txt *Text) Clear(options ...func(*Text)) {
txt.prevR = -1
txt.bounds = pixel.Rect{}
txt.tris.SetLen(0)
txt.dirty = true
txt.Dot = txt.Orig
for _, option := range options {
option(txt)
}
}
// SetDot is an option to set the Dot field.
func SetDot(dot pixel.Vec) func(*Text) {
return func(txt *Text) {
txt.Dot = dot
}
} This could then be used like this: p.txt.Clear(text.SetDot(p.txt.Dot)) |
I usually prefer to skip the |
Hm, I think adding the options to |
And currently, there's no question about the |
Oh and one more thing. p.txt.Clear(text.SetDot(p.txt.Dot)) is the same number of characters as p.txt.Clear()
p.txt.Dot = p.txt.Orig So you woudn't save any typing, you'd just squash it in one line. |
But that last example wouldn't do the same thing. I just gave an option for the less common case where you want to manually set the Dot field to something that is changed earlier in the Clear method. Oh, and |
Ah, I was reading it incorrectly. Anyway, still I think it's much clearer and simpler the 3 line way: dot := txt.Dot
txt.Clear()
txt.Dot = dot Considering how unusual this is, I think this is sufficient. |
*text.Text
would allow us to not having to do this:Adding a
Printf(format string, a ...interface{}) (n int, err error)
method on*text.Text
would allow users of the github.com/faiface/pixel/text package to not directly import
fmt
I'd also like us to consider adding a default atlas (
text.BasicAtlas
) initialized by:I believe that doing these changes to the text package would make it more convenient to use. Do you agree @faiface?
The text was updated successfully, but these errors were encountered: