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

Weapon sheathing #1615

Merged
merged 1 commit into from Nov 8, 2018
Merged

Weapon sheathing #1615

merged 1 commit into from Nov 8, 2018

Conversation

akortunov
Copy link
Collaborator

@akortunov akortunov commented Feb 25, 2018

Implements feature #4673.
Related topic is here.
Here are WIP animations for this feature: Animations.zip

Basic idea: add some new bones to humanoid skeleton and attach weapons/quivers/scabbards/etc to these bones.

Data format is compatible with the Weapon Sheathing mod, so you can use meshes from this mod.
xbase_anim_sh.nif is required, other files are optional.

Sheathed models for now are separate meshes with _sh suffix. If there is no scabbard for current weapon mesh, an engine will show only holstered weapon. Also scabbards and quivers do not have enchanting glow (but arrows in quiver do).

The feature is disabled by default. You should enable it in config file:

[Game]
weapon sheathing = true
use additional anim sources = true

Now this feature supports:

  1. Weapon holstering.
    Holstered weapons also has glow when enchanted.
  2. Scabbards for any type of weapon excepts of throwing.
    Scabbard itself does not have a glow.
  3. Quivers for bows and crossbows.
    Quivers itself do not have a glow, but arrows have when enchanted.
    Count and type of rendered arrows depends on type and count of equipped arrows.

Notes:

  1. I had to rework autoequipping
  2. Shield holstering is not supported for now - there is a lot of issues with resources (we will need to add new animation groups, there is a lot of clipping with movement animations, issues with mods that use pseudo-shields, etc).

@Capostrophic
Copy link
Collaborator

Capostrophic commented Feb 25, 2018

Concerning quivers: in Skyrim, it's a bit complex. There are two ammo meshes you need for setting a ranged weapon up: one is for quiver with five static arrows (their IDs in meshes are numbered so the engine knows which not to enable if the character only has 4 or less arrows) and one arrow which the character always gets out from the quiver during archery animations, and one is for a single arrow. In content files there are two records for an arrow/bolt: one (AMMO) defines arrows of a certain type in general and another (PROJ) defines the flying arrow projectile. The first mesh is assigned to the first record. The projectile mesh is assigned to the second.
The quiver with arrows is attached to QUIVER node of the skeleton. If the player equips certain ammunition, it appears there and will not disappear unless the player unequips the ammunition or loses all arrows (NPCs normally don't spend ammo (technically; they still shoot) or unequip arrows if they have them). Well, duh.

@Thunderforge
Copy link
Contributor

Thunderforge commented Feb 25, 2018

That's all - no scripting, no every NIF conversion, no ESP.
Just install two meshes as a common ESP-less mod and move around the world.

So this is going to be a mod eventually? I'm confused as to why this needs to be a PR for the game.

Would it make sense to configure it somehow so that it can be a settings.cfg setting without a mod?

@kcat
Copy link
Contributor

kcat commented Feb 25, 2018

So this is going to be a mod eventually? I'm confused as to why this needs to be a PR for the game.

It will need a mod to work, but it also needs engine-side support. The engine needs to know that if an NPC has certain nodes, it should render equipped weapons on them when sheathed. The default nifs don't contain those nodes so users won't see any change automatically -- someone will need to make a mod with the necessary changes to see a difference (provided separately, like on Nexus).

@akortunov akortunov changed the title [WIP, discussion] Weapon sheating [WIP, discussion] Weapon sheathing Feb 26, 2018
@akortunov
Copy link
Collaborator Author

akortunov commented Feb 26, 2018

After additional testing I noticed that crossbowmans in Morrowind use a some kind of ammo belt instead of quivers, judging by reload animation.

So maybe I will have to split quivers and ammo belts to separate feature.

Anyway, @scrawl, @zinnschlag, what do you think about weapon sheathing?

@akortunov
Copy link
Collaborator Author

akortunov commented Feb 26, 2018

Also can anyone explain why it does not work for beast races and bipedal creatures?
I thought the engine supposed to load xbase_anim.nif for them too, but the NodeMap does not contain new nodes.

I suppose for beast races an engine uses the xbase_animkna.nif, right? But how does the engine handle creatures? Uses per-creature model?

@kcat
Copy link
Contributor

kcat commented Feb 26, 2018

Also can anyone explain why it does not work for beast races and bipedal creatures?

IIRC, beast races use a different skeleton, one which includes tail bones that don't exist in the normal biped skeleton. They have the suffix "_kna" (Khajiit 'n Argonian).

For biped creatures, I'm not sure. The model and animation handling for creatures and NPCs are in different classes, but since biped creatures share some NPC-like behavior with regard to some equipment like weapons, I don't know how exactly they're handled.

@akortunov
Copy link
Collaborator Author

akortunov commented Feb 27, 2018

I uploaded the updated version:

  1. Creatures support. Unfortunately you need to edit mesh for any creature you want to suppport weapon sheathing. Also I noticed that creatures do not autoequip weapons after spawn, they equip only shield. But it is a separate bug.
  2. Refactoring
  3. Drop quivers support for now since we do not have the design yet.

@testman42
Copy link

Would it make sense to configure it somehow so that it can be a settings.cfg setting without a mod?

If this is going to be part of OpenMW codebase, user should be able to toggle this behavior in settings / in launcher.

@akortunov
Copy link
Collaborator Author

akortunov commented Feb 27, 2018

If this is going to be part of OpenMW codebase, user should be able to toggle this behavior in settings / in launcher.

Again, this setting will do nothing if the player does not have modded assets (xbase_anim* at least).
This feature required support by both assets and engine.

UPDATE: I uploaded new assets.
Now this feature works for player, all NPCs and dremoras.

Also related question: is it possible to use one file per animation instead of packed xbase_anim?

@akortunov
Copy link
Collaborator Author

akortunov commented Feb 27, 2018

Is there any way to do not edit every creature model to add new bones?

@ghost
Copy link

ghost commented Feb 27, 2018

I haven't really reviewed this, so just a general comment for now:
The main concern with a feature like this, adding new meanings for node names or filenames has an ever so small chance of something triggering that name by accident. Just for that reason e.g. the 'auto normal maps' feature is also disabled by default.
In any case we should be sure the feature is perfect before we roll it because people will start making mods and any further updates we make will probably break those mods.
It's a bit comparable to introducing new scripting functions before 1.0 which, for the record, Zini seems to be against.
Do you think the feature could be cleaner if e.g. the ESM format was updated so you can make new body part types?

Re: the last comment, I don't see another way than editing models because I'm sure the location you want at depends on the model.

@akortunov
Copy link
Collaborator Author

akortunov commented Feb 28, 2018

Re: the last comment, I don't see another way than editing models because I'm sure the location you want at depends on the model.

What kind of model? Weapon or creature ones? For example, sheathed weapon may have different position for humanoid NPCs and beast ones.

Do you think the feature could be cleaner if e.g. the ESM format was updated so you can make new body part types?

As for me, new bodyparts are good for armor and clothing (for example, if we will want to show jewelry). Not sure if bodyparts will help with weapon sheathing since creatures do not have bodyparts at all.

@akortunov
Copy link
Collaborator Author

A bit offtopic question: why do not add an ability to load animations from per-group .kf files? Performance issues?

It would be nice to provide a couple of modded holster animations for weapon sheathing without replacing the whole xbase_anim.kf

@akortunov akortunov force-pushed the holstered_weapons branch 2 times, most recently from 73de96e to 7b2ef63 Compare March 3, 2018 10:18
@akortunov
Copy link
Collaborator Author

akortunov commented Mar 3, 2018

Update:

  1. Make this feature optional
  2. Squash commits
  3. Merge upstream changes from master.

This feature is basically implemented. Now we need an independent testing.

@akortunov akortunov changed the title [WIP, discussion] Weapon sheathing [discussion] Weapon sheathing Mar 3, 2018
@akortunov
Copy link
Collaborator Author

How it works for scabbards:

image

@akortunov
Copy link
Collaborator Author

akortunov commented Mar 25, 2018

Added quivers support:
222
Quiver is a part of bow, similarly to scabbard.
Type and count of arrows in quiver depends on count and type of equipped arrows.

Note: autoequipping ignores ammo, so quivers will be empty for NPCs before first combat.
If we will force NPCs to autoequip ammo, it may alter item list for some traders (PR #1614).

@akortunov
Copy link
Collaborator Author

akortunov commented Apr 1, 2018

it may alter item list for some traders

I modified autoequipping, now I just ignore restockable ammo.

@Fiona-J-W
Copy link
Contributor

I think it would probably be better to have quivers as their own item-category and require their use to be able to shoot any bow (either at all or with reasonable speed). That way the quiver would also not be auto-removed/replaced on a weapon-change. It would also create a couple of further interessting-possibilities, like:

  • Quivers with maximum capacity: You have to refill the quiver from the inventory after you used up all their arrows. Different quivers offer different capacities.
  • Quivers with different ease of drawing an arrow: Some result in faster reloads than others.
  • Quivers with different weights.
  • Maybe even different durability?

All that would create interessting choices for the player. It would also avoid the issue that pretty much every bow that you pick up gives you a quiver that is not visible when it is lying around.

Note that I'm not suggesting the same for scabbards, as those have to be build with the specific sword in mind one way or another.

@Fiona-J-W
Copy link
Contributor

Oh, and having shields shouldered when you are not in combat would also be awesome.

@akortunov
Copy link
Collaborator Author

akortunov commented Oct 26, 2018

Am I doing something wrong?

Are you sure that you installed meshes from Weapon Sheathing MWSE mod?
As a minimum requirement, you need a xbase_anim_sh.nif.

A quote from documentation:

Basically, player needs a xbase_anim_sh.nif file and optional _sh files for quivers and scabbards.

@IriaSomobu
Copy link

No. There's nothing in first post about it.

Okay, I'm going to install it.

@IriaSomobu
Copy link

Yaay, it's working!

screenshot003

@akortunov akortunov force-pushed the holstered_weapons branch 2 times, most recently from fe1dd85 to 5f012a4 Compare October 30, 2018 04:29
@psi29a
Copy link
Member

psi29a commented Oct 30, 2018

Ready for 0.46? :)

@akortunov
Copy link
Collaborator Author

Ready for 0.46? :)

Since I did not get additional bugreports, I suppose yes.
But I'd prefer to fix build issues caused by recastnavigation.

@akortunov
Copy link
Collaborator Author

I suppose we mostly fixed the RecastNavigation mess, right?

@psi29a
Copy link
Member

psi29a commented Nov 3, 2018

The dust has mostly settled.
@AnyOldName3 You ready? :)

@AnyOldName3
Copy link
Member

I don't think I have any particular objections to merging this.

@IriaSomobu
Copy link

I haven't faced with bugs. But I've played without mods.

@psi29a
Copy link
Member

psi29a commented Nov 4, 2018

Just for the record, this PR also resolves one of the last warnings being triggered by openmw in msvc 2015/2017.
#2019 (comment)

@akortunov
Copy link
Collaborator Author

So what did we decide? Are there issues which I need to address, or we can merge this PR?

@akortunov
Copy link
Collaborator Author

Updated PR by changes requested by Capostrophic.

@psi29a
Copy link
Member

psi29a commented Nov 6, 2018

Last call before merging... any other comments or reviews?

@akortunov
Copy link
Collaborator Author

Last call before merging... any other comments or reviews?

I suppose there was enough time to get feedback.

@psi29a
Copy link
Member

psi29a commented Nov 8, 2018

Merged!

@psi29a psi29a merged commit f6243fa into OpenMW:master Nov 8, 2018
@psi29a psi29a removed the On hold label Nov 8, 2018
@@ -51,6 +63,302 @@ ActorAnimation::~ActorAnimation()
{
mInsert->removeChild(iter->second);
}

mScabbard.reset();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a purpose to call reset here? C++ compilers generate code to autmatically destruct all members.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet