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

Refactor weapon types behaviour #2413

Merged
merged 4 commits into from Aug 9, 2019

Conversation

@akortunov
Copy link
Collaborator

akortunov commented Jun 6, 2019

Replacement of #2134.

Since we do not have a good way to de-hardcode settings right now, just de-hardcode the custom rigs loading and still move weapon settings to the table. Later we can add Lua bindings to it, if needed.

Even in its current state this PR allows to define different placement of sheathing bones for actors with different skeletions (males, females and beast races).
Testing data (animations + skeletons): Data.zip

Once this PR will be ready for merging, I'll contact with Greatness7 to update his mod and update documentation.

apps/openmw/mwclass/armor.cpp Show resolved Hide resolved
apps/openmw/mwclass/weapon.cpp Outdated Show resolved Hide resolved
apps/openmw/mwrender/npcanimation.cpp Outdated Show resolved Hide resolved
apps/openmw/mwworld/projectilemanager.cpp Outdated Show resolved Hide resolved
@akortunov akortunov force-pushed the akortunov:weapon branch from d49e09f to bb07b2c Jun 9, 2019
@akortunov

This comment has been minimized.

Copy link
Collaborator Author

akortunov commented Jun 20, 2019

@Capostrophic, @AnyOldName3, @kcat, @wareya, any objections or suggestions?

@AnyOldName3

This comment has been minimized.

Copy link
Member

AnyOldName3 commented Jun 20, 2019

No, but that doesn't necessarily mean I have any idea what's going on with this, as it's mostly mwmechanics, which I haven't done much with.

@akortunov

This comment has been minimized.

Copy link
Collaborator Author

akortunov commented Jun 20, 2019

No, but that doesn't necessarily mean I have any idea what's going on with this, as it's mostly mwmechanics, which I haven't done much with.

Well, first of all, there could be general codestyle issues.
Second, I use DYNAMIC data variance for injected rigs (we seems to use it for other rigs as well, but I can be wrong here). IIRC, you do not like such things.
Third, I wanted to make this table configurable in some way, but I am not sure if we have good pre-1.0 solutions here. But it would be nice to have a bow in left hand (I have an almost full animation set for this feature) or Skyrim-style blunt weapons sheathing (which requires a separate animation group for axes and maces).

@AnyOldName3

This comment has been minimized.

Copy link
Member

AnyOldName3 commented Jun 20, 2019

Second, I use DYNAMIC data variance for injected rigs (we seems to use it for other rigs as well, but I can be wrong here). IIRC, you do not like such things.

Is it on a drawable or stateset? If not, it's a flag for the optimiser rather than a flag for rendering, so doesn't actually break things.

@akortunov

This comment has been minimized.

Copy link
Collaborator Author

akortunov commented Jun 20, 2019

Is it on a drawable or stateset?

On the drawable.

@AnyOldName3

This comment has been minimized.

Copy link
Member

AnyOldName3 commented Jun 20, 2019

Are you sure? The node is a Node pointer, not a Geometry pointer, so I'd be a little surprised if it was actually the drawable.

@akortunov

This comment has been minimized.

Copy link
Collaborator Author

akortunov commented Jun 20, 2019

Are you sure? The node is a Node pointer, not a Geometry pointer, so I'd be a little surprised if it was actually the drawable.

Well, at least on the loaded node itself. I doubt that every bone node has its own stateset.

Anyway, I can get rid of DYNAMIC here if there is a better way to tell optimizer to do not try to optimize injected bones. Of course, I can just write to docs that any injected node should have a name starting with "Bip01", but I am not sure if it is a good solution.

@AnyOldName3

This comment has been minimized.

Copy link
Member

AnyOldName3 commented Jun 20, 2019

When I say

Is it on a drawable or stateset?

I'm asking is it on

  • A drawable or a stateset.

or

  • Some other kind of node.

If it's a Group or a Transform or whatever, it's not a drawable.

@akortunov akortunov force-pushed the akortunov:weapon branch from bb07b2c to 5dce55d Jun 21, 2019
@akortunov

This comment has been minimized.

Copy link
Collaborator Author

akortunov commented Jun 21, 2019

Anyway, I removed DYNAMIC. Users will need to use bones with reserved names.

@AnyOldName3

This comment has been minimized.

Copy link
Member

AnyOldName3 commented Jun 21, 2019

If it isn't on a drawable, there's no problem with using dynamic, so if it makes things better, don't remove it.

@Capostrophic

This comment has been minimized.

Copy link
Collaborator

Capostrophic commented Jul 31, 2019

Nothing obvious coming up. I think it's time to prepare for this to be merged?

@akortunov akortunov force-pushed the akortunov:weapon branch from 5dce55d to 277b68f Aug 9, 2019
@akortunov

This comment has been minimized.

Copy link
Collaborator Author

akortunov commented Aug 9, 2019

I think it's time to prepare for this to be merged?

Which kind of preparation is needed?

@akortunov akortunov force-pushed the akortunov:weapon branch from 277b68f to fa32b46 Aug 9, 2019
akortunov added 4 commits Dec 26, 2018
1. Move weapon types behaviour from switches to the table (should allow
us to de-hardcode weapon types later)
2. Gereralize bones injection to actors skeletons (instead of using the
hardcoded xbase_anim_sh.nif)
@akortunov akortunov force-pushed the akortunov:weapon branch from fa32b46 to fcd6e91 Aug 9, 2019
@Capostrophic Capostrophic merged commit 307e9ba into OpenMW:master Aug 9, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@akortunov akortunov deleted the akortunov:weapon branch Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.