Skip to content
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

Console API rework #140

Merged
merged 4 commits into from
Apr 6, 2015
Merged

Console API rework #140

merged 4 commits into from
Apr 6, 2015

Conversation

zsparal
Copy link
Collaborator

@zsparal zsparal commented Apr 6, 2015

This still has the names OffscreenConsole and RootConsole. I'm kind of partial to these, since you can import them nicely from the console module and still know what they do.

If the naming is fine as it is, then this closes #137

It returns a mutable pointer without requiring &mut self. For the implementation's sake this is the ideal behaviour, but this still means that calling it is an unsafe operation. Unfortunately there's no way to remove this from the public interface without crippling console::blit, as this requires the underlying native representation.
@tomassedovic
Copy link
Owner

I'd prefer Offscreen and Root names in the console module. If someone wants the imported names to be more explicit, they can always say use tcod::console::Root as RootConsole.

And we can do that re-export in lib.rs, too. What do you think?

Otherwise, this looks great. For the record, making Console::con unsafe makes sense and I actually like that it's public -- people can use it to call the low-level FFI stuff if the want (which is inherently unsafe, too).

@zsparal
Copy link
Collaborator Author

zsparal commented Apr 6, 2015

Yes, I like the public Console::con as well, but it would've been more elegant if I could've implemented it using the AsNative traits. But it's not really feasible if we want to keep the current blit semantics (which are more important than the unsafe Console::con).

tomassedovic added a commit that referenced this pull request Apr 6, 2015
@tomassedovic tomassedovic merged commit f62c56a into tomassedovic:master Apr 6, 2015
@zsparal zsparal deleted the console_api branch April 6, 2015 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console API
2 participants