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

whitelist a handful of rendering-related engine and library classes #2463

Closed

Conversation

kaen
Copy link
Contributor

@kaen kaen commented Sep 1, 2016

Contains

Whitelist entries for low-level rendering classes like org.lwjgl.opengl.GL11

How to test

Just try to access one of the listed classes :)

My WIP HealthBars module requires this patch to run: https://github.com/kaen/HealthBars/blob/master/src/main/java/org/terasology/healthbars/HealthBarRenderSystem.java#L194

I looked for any previous discussion of whitelisting these classes and couldn't find any, hopefully there's not an obvious security-related reason for excluding them.

@GooeyHub
Copy link
Member

GooeyHub commented Sep 1, 2016

Can one of the admins please verify this patch?

@Cervator
Copy link
Member

Cervator commented Sep 1, 2016

Cool to see the health bar module :-)

Generally we restrict modules from direct access to rendering stuff (think visual cheats or forced crashes from invalid rendering). It may be better to include the health bar rendering directly as engine classes then allow modules to hook them up. How does that sound?

@kaen
Copy link
Contributor Author

kaen commented Sep 1, 2016

Yeah I was just reading up on #2449 and realized the issue is more complicated than I thought when I opened this PR.

That sounds fine for this case but it's pretty common to want to render a billboard, in fact I have another WIP module that would really want a similar capability and definitely doesn't belong in the engine. I know we spoke about some central system for that on Slack.

I'm certainly not knowledgeable about rendering-related security issues, but it seems to me that one could just as easily force a crash via many large memory allocations, and that a cheater could locally alter his build arbitrarily to circumvent sandbox protections.

It also seems unsustainable to require new graphical functionality to be exposed via a component system. A new system for billboards, another for render-to-texture, a third for shader manipulation, and so on until we have a ComponentSystem-based re-implementation of OpenGL :P

However, I again reiterate that I am speaking out of school, and defer to more knowledgeable developers on all these points. I'm happy to move HealthBars into the engine when it matures and move forward with other modules on a case-by-case basis.

@kaen kaen closed this Sep 1, 2016
@Cervator
Copy link
Member

Cervator commented Sep 1, 2016

We definitely need more rendering power in modules, no argument there, and the pro/con list is more ambiguous than just "no access no problem!" :-)

The nice thing is that we can just add a few more super useful but rendering heavy things to the engine with a goal to extract it out later. That's not a problem at all IMHO. Other things are still sitting in the Core module waiting to be moved out later. So by all means throw a few nice utilities that way for now, put them to use in modules, and we'll keep tweaking at the ideal setup until we get there.

@skaldarnar
Copy link
Member

A question on the side: don't we have health bars (-> hunger, thirst https://github.com/Terasology/Hunger) already, where the texture can be tweaked via asset? Or are you working on some advanced health bar system?

@kaen
Copy link
Contributor Author

kaen commented Sep 1, 2016

These are for mobs, not a first-person health bar. The need for the GL functions over simply using a UI element is so that health bars respect Z-ordering and world lighting in order to not give away the position of an attached mob.

@skaldarnar
Copy link
Member

Ah, I see. So the result should be like the name tags we currently have, floating over a mob? This reminds me of some old thread on UI, especially some augmented in-game UI stuff. Let me see if I can find it....

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.

4 participants