Newbrict / engi

Oaktales engi fork
BSD 2-Clause "Simplified" License
0 stars 0 forks source link

Font Interface #24

Open paked opened 9 years ago

paked commented 9 years ago

The new font interface is a bit of a mess to interact with. The following should probably be added:

Newbrict commented 9 years ago

I don't know how I feel about splitting font/text but I think making rendering easier makes sense, wasn't sure on what the best approach is though. what would be the point of newcolor and newmethod?

paked commented 9 years ago

Oh I must have gotten distracted while writing that...

Newbrict commented 9 years ago

I don't think we need a Text.Render func, they're just textures so they can piggyback on that.

On second thought I don't think it makes sense to split it up into text and font anyway, you use a font to render a text.

paked commented 9 years ago

Well your current approach relies on having the text when the font/text is being rendered. The only way t currently do this would be to store it within the Font struct.

The Font and Text should be split up. A Font is a set of characters loaded from a TTF file, they have no idea of what order letters should be processed in/etc. Text is a string of characters specified by a component, it has no idea what the characters look like... But it does know which font to render with.

Also with your current implementation as shown in OakTale, I am not quite sure how I would change the rendered text.

Newbrict commented 9 years ago

You don't have to store the string in the font struct. You create the font, then you render a string using that font. take a look at OakTale/new_font branch.

to change the rendered text you just font.Render("a different string") it has to be re rendered.

paked commented 9 years ago

As an experiment, I went and implemented what I thought the interface would be better as. It is available now in https://github.com/Newbrict/engi/tree/example_text and https://github.com/Newbrict/OakTale/tree/example_text

Event if you do not like these changes... Please keep the NewFont func, the current way is a bit too C-like ;)

Newbrict commented 9 years ago

I think NewFont makes sense, it disambiguates the ttf field and is more easily documentable.

What's the reason changing Render(..) to Texture(..)? I think separating it into Text makes it more complicated, do you honestly think it makes it more clear?

paked commented 9 years ago

I changed Render(...) because we are already using a function called Render as part of the Renderable interface, it was just to remove ambiguity.

Yes. Maybe ask the other contributors for their opinion?

TheBerryBeast commented 9 years ago

One of the gui components I created is similar to your implementation of your text struct so i do not think it will be necessary.

On Wed, Jun 3, 2015 at 7:43 PM, Harrison Shoebridge < notifications@github.com> wrote:

I changed Render(...) because we are already using a function called Render as part of the Renderable interface, it was just to remove ambiguity.

Yes. Maybe ask the other contributors for their opinion?

— Reply to this email directly or view it on GitHub https://github.com/Newbrict/engi/issues/24#issuecomment-108647876.

paked commented 9 years ago

Is this code uploaded?

TheBerryBeast commented 9 years ago

It is not but I will commit and pull request it very soon.

On Wed, Jun 3, 2015 at 8:19 PM, Harrison Shoebridge < notifications@github.com> wrote:

Is this code uploaded?

— Reply to this email directly or view it on GitHub https://github.com/Newbrict/engi/issues/24#issuecomment-108655250.

paked commented 9 years ago

Ok, well I'll check out your implementation then

Newbrict commented 9 years ago

The reason I called it render is because it literally renders the texture of the string from the font, maybe it could be renamed to RenderText() or something