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

Shields, newer-better-meshier... #2491

Closed
wants to merge 6 commits into from

Conversation

fluffyfreak
Copy link
Contributor

Description:

Mesh shields, 2nd attempt based on the aborted #2139 and feature request #2013.
This is a much simpler version, no textures, a very basic shader and as little hacking as I could manage.

Problems:

This is subjective of course but there are parts I'm not sure I made the right choices:

  • The Scenegraph manipulation - although it might be cleaner keeping specific information out of the scenegraph it did mean that I had to fettle with it from outside to create a new Group for the shield meshes, then de-parent and re-parent the StaticGeometry - it all felt a bit hacky.
  • Creating the shields repeatedly - I feel like this could almost be cached, but there is some specific information (colour) for each ship so I'm not so sure, also they need to point that that instance of the mesh rather than a generic one, I think.

Functionally it all seems to be correct and working, I need to add a better way of accessing the colour, and it does still "behave" in the same way as the old shields - charging up to full power and then vanishing, but no-ones said what they'd like to see done about that so I've left it as-is.

Andy

@Zordey
Copy link
Contributor

Zordey commented Oct 16, 2013

It looks like you are using shaders for the new shields?

What happens if you have shaders disabled in game? Will it revert to the old (current) form, or does it still use the mesh with basic colours?

@fluffyfreak
Copy link
Contributor Author

@Zordey At the moment I just haven't cared about non-shader problems. Hadn't even thought about it as even the problems that the ATi X1950 has had with the game are still capable of using shaders.

@fluffyfreak
Copy link
Contributor Author

Obviously, do not try and merge ;)

@robn
Copy link
Member

robn commented Oct 29, 2013

#2493 was just merged and probably breaks what you're doing a bit. Though it occurs to me that you might be able to use it. ModelBodies can now have multiple collision meshes, so the shield could just be a magic one that overrides the others when they're up. Not that I've given it more thought than what was required to write this paragraph :)

@fluffyfreak
Copy link
Contributor Author

Sadly I'm really struggling with the collision stuff :(
Not to mention the shields now crash whenever a new ship gets added to the scene :( :( :(

@fluffyfreak
Copy link
Contributor Author

In the interests of splitting this into separate commits for the rendering and the shield collisions I've removed the collision mesh work from this commit.

The hit rendering effect is still present and it's visible in certain cases where the collision mesh is very close to the shield geometry, also it's clearly visible in the modelviewer.

@fluffyfreak
Copy link
Contributor Author

Bug fixed, memory trampling due to stupidity.

@fluffyfreak
Copy link
Contributor Author

I got this compiling and running on my Linux Mint 15 machine this morning before work.
If you just want to test out the hit effect or see the shield on a ship then I recommend -mv deneb as your first one to check out - note that not all of the ships have shield meshes. A couple of new ones have gone in so a couple are missing now I think.

@robn that should save you rebooting into Windows ;)

@fluffyfreak
Copy link
Contributor Author

Brought up-to-date with master.

@fluffyfreak
Copy link
Contributor Author

Updated to master again, no troubles merging this time :)
Also fixed a comment.

I have no plans to work on this any further until this part is merged in or I've got some feedback about it.

@robn
Copy link
Member

robn commented Dec 14, 2013

Not forgotten, I promise.

@fluffyfreak
Copy link
Contributor Author

It's ok I'm not complaining, we do seem to be really reviewer bound at the moment :) especially with me backporting stuff from Paragon.

@fluffyfreak
Copy link
Contributor Author

Re-merged, fixed conflicts etc.

@robn
Copy link
Member

robn commented Jan 14, 2014

GCC gave some sign-compare and unused-variable warnings, which I've fixed. That's on robn/shields. Does MSVC have any equivalent warnings that can be enabled?

The shields appear to be oriented incorrectly. I guess its supposed to be facing the camera?

snap

snap1

Some of the newer ships don't have shield meshes, and so display nothing. Is there an easy/obvious way to create a mesh for something that doesn't have one? Or should we just require it for all ships that can equip shields? I'm inclined to just say "require it". @nozmajner, do you have an opinion? Can you provide meshes for the other ships we have?

The "hit it" button in the modelviewer trips an assert if the model doesn't have a shield.

ShieldRenderParameters probably belongs in Shields.h.

Does the Shields class belong under SceneGraph::? I'm honestly unsure. It manipulates the mesh which makes me think its something akin to SceneGraph::Loader. But it does is safely from a distance, and is highly game-specific, so maybe it doesn't. Then again, SceneGraph::Thruster is game-specific too. @Luomu, do you have a good idea where something like this fits into SGM's design? Otherwise it can stay where it is.

@fluffyfreak
Copy link
Contributor Author

Weird the orientation is ok for me. Looking into it now.

@nozmajner
Copy link
Contributor

I checked, the meshes have their orientations properly, Y+ fwd, X- left Z+ up.
I think we should require it, it's not hard to create them. I'll try using a subsurfed collision mesh to sew how that works, but I think it's better to do them by hand.
I'll cook up the missing ones quickly.
https://dl.dropboxusercontent.com/u/456051/pioneer/shield_meshes.zip

The Nerodia and Sinonatrix were missing them, also the two versions of the Kanara and the Malabar/Vatakara pair can share them.
I'm not sure about the optimal poly count. Some of them seems a bit blocky, like the Natrix above, with 176 tris, but the 1280 tri of the malabars seems to be a bit too much.

@fluffyfreak
Copy link
Contributor Author

Ah, found it :) that unused-variable was just there whilst it gets the accumulated and transformed matrix of the node.

@fluffyfreak
Copy link
Contributor Author

@nozmajner Some of the hardness I can probably counter in the shader, will have to see.

@robn
Copy link
Member

robn commented Jan 14, 2014

Crap, it had output variables. Sorry, I should have looked more closely,

@fluffyfreak
Copy link
Contributor Author

Easy to miss, I shouldn't have left it there.

@fluffyfreak
Copy link
Contributor Author

Ok, fixed a bug with pressing the hit it button when there's no shield, then just disabled the buttons in case there's no shield :)

@robn
Copy link
Member

robn commented Jan 15, 2014

Great! I've got the code and the new meshes onto my laptop. I'll put it all together and merge it on my train ride. Cheers!

@fluffyfreak
Copy link
Contributor Author

Too late!!!

@robn
Copy link
Member

robn commented Jan 15, 2014

Hah. Ok then, backing out the merge that I hadn't pushed yet because my network on the train crapped out ;)

@robn
Copy link
Member

robn commented Jan 15, 2014

Meh, bollocks to it. I wrote the changelog already. Our model update commits are the same, so I've taken my merge to master, picked @93dbfc0 onto it and pushed. Job done!

@robn robn closed this Jan 15, 2014
@fluffyfreak
Copy link
Contributor Author

Excellent, next improvement will be to figure out how to get things to actually hit the shield mesh itself!
Cheers @robn

@fluffyfreak fluffyfreak deleted the Shields branch January 16, 2014 12:21
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.

None yet

4 participants