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

Make thrown projectiles rotate (#4216) #1563

Merged
6 commits merged into from Nov 24, 2017
Merged

Make thrown projectiles rotate (#4216) #1563

6 commits merged into from Nov 24, 2017

Conversation

drummyfish
Copy link
Contributor

@drummyfish drummyfish commented Nov 23, 2017

Attempt to make thrown projectiles rotate, like they do in vanilla (video of the bug). I added two fields one field to projectile state and added code to save and load the new state to/from savegames, but I need someone to check I did it right.

We could discuss:

  • speed of rotation - here's how it looks now, see video description for footage of different speeds
  • center of rotation: The projectile now rotates around its tip, but it's not much noticeable because it rotates slowly, I could shift the center of rotation but I would probably need to choose some constant (or maybe center of bounding box?)

@akortunov
Copy link
Collaborator

Looks like this PR changes the savegame format (as a #1559), so old game versions will not load new savegames.

@drummyfish
Copy link
Contributor Author

Yes it changes the format, the new version still loads the old savegames though.

Can't we make it so that the game skips the unknown records when loading so that this isn't an issue next time we modify the format?

@akortunov
Copy link
Collaborator

Can't we make it so that the game skips the unknown records when loading so that this isn't an issue next time we modify the format?

A good question. But I think OpenMW does not do that for a reason.

@drummyfish
Copy link
Contributor Author

Actually I can use mEffectAnimationTime for the animation, gotta fix that.

@drummyfish
Copy link
Contributor Author

drummyfish commented Nov 23, 2017

Actually I'm doing this all wrong, there already is RotateCallback I should probably use.

EDIT: I'm using the callback and the rotation is wrong due to the quaternion rotation above it that f*cks with the roll as I mention in the bugtracker, so it's not that easy. Rotations that aren't along the axis of flight simply don't work. Maybe that's why the person who was implementing it left the thrown projectile rotation out. My solution works, but maybe I should make it more consistent and make a separate callback for thrown projectiles?

@ghost
Copy link

ghost commented Nov 23, 2017

Forward compatibility break would be OK here, same as #1470: Projectiles are short-lived and you're unlikely to save your game during battle. In all other cases there are no such records in the save-game.

With that said, do we need to store this state at all? We can just check the type of weapon on loading a game.

@ghost
Copy link

ghost commented Nov 23, 2017

By the way, here's a somewhat related issue: https://bugs.openmw.org/issues/3778

@drummyfish
Copy link
Contributor Author

We can just check the type of weapon on loading a game.

Done.

@drummyfish
Copy link
Contributor Author

drummyfish commented Nov 23, 2017

I think the vanilla developers didn't bother very much with thrown weapons, throwing stars rotate like this (about the red axis):

image

My projectiles are rotated the "correct" way. Is it OK or should I copy the original behavior?

@drummyfish
Copy link
Contributor Author

drummyfish commented Nov 23, 2017

Here is how I'm rotating them now (in the video I'm trying to center the pivot point according to bounding sphere which somehow works except for stars).

@drummyfish
Copy link
Contributor Author

All projectiles now rotate nicely around the center of their bounding box :)

@drummyfish
Copy link
Contributor Author

drummyfish commented Nov 23, 2017

Looks kinda good now I think :) Should I do something else with the code? For these thrown projectiles I'm mixing the computation of general orientation with rotating it, so I don't know if I should try to move the computation into a separate update callback subclass, seems like it would be adding more unnecessary code and classes.

@ghost
Copy link

ghost commented Nov 23, 2017

LGTM. I'll just give it a test now.

@@ -202,6 +203,12 @@ namespace MWWorld

osg::ref_ptr<osg::Node> projectile = mResourceSystem->getSceneManager()->getInstance(model, attachTo);

osg::ref_ptr<osg::ComputeBoundsVisitor> boundVisitor = new osg::ComputeBoundsVisitor();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, visitors can be created on the stack, so that line can be shortened to:

osg::ComputeBoundsVisitor boundVisitor;

You can do that with many other OSG objects, as long as you never create a ref_ptr of them.

@ghost ghost merged commit f5c075b into OpenMW:master Nov 24, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants