-
Notifications
You must be signed in to change notification settings - Fork 38
QoL stuff for pre-5 and the future #480
Conversation
…be run in tandem with `MovableMan` function `ChangeActorTeam` so that the player won't control an actor of the wrong team. (TODO: Allow modders to change actor team without losing control?)
… describe any method of delivery, such as teleportation
…erited from the parent actor), so that `OnDetach` can run assign scripted velocities to the dropped item (Uzira sword)
…been flagged for deletion (TODO: settings flag?)
…roperly raises the `AboveHUDPos`
…ht of a column of scrap terrain that can get collapsed. Also cleaned up around compacting height code, and made the pixels falling from the column less erratic.
…illations` and `AngOscillations` which are frankly crap methods of detecting stillness, when pos and angle shifts are already being read.
garethyr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions and changes
Dropped items now need to be still in order for the HUD to show, making actor deaths less noisy visually
…ng `AddedActors`, however removing the `Actors.empty()` check is enough since `ActorRoster` is the only relevant thing in the function
…er conversion, fix spelling, add Lua getter
garethyr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor things
Entities/AHuman.cpp
Outdated
| if (!m_ActivateBGItem) { | ||
| if (deviceAsFirearm->IsFullAuto()) { | ||
| deviceAsFirearm->Activate(); | ||
| m_ActivateBGItem = (deviceAsFirearm->FiredOnce() || deviceAsFirearm->IsReloading()) && deviceAsFirearm->HalfwayToNextRound(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to me the logic should be
m_ActivateBGItem = (deviceAsFirearm->FiredOnce() && deviceAsFirearm->HalfwayToNextRound()) || deviceAsFirearm->IsReloading();Otherwise a gun with a very slow fire rate will lock out firing the bg gun while it's reloading, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested it pretty thoroughly and it seemed to work fine, I guess I can give it another whirl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thanks. I may very well have been wrong haha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked into it, I was technically right. Your code there would brick the bg gun if the fg one is reloading and has a very low ROF
However, you have code on line 3358 that sets the bg gun activation flag to true if the fg gun is reloading.
I've cleaned up the reloading part of this stuff to make it more simple and clear. Feel free to resolve this (just leaving it open so you can read it if you want)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, also, while testing I noticed that the bg item can freely activate if you keep the fire button held down through the reload. E.g. if you equip 2 guns, fire a shot, run for a in MovableMan.Actors do ToHDFirearm(ToAHuman(a).EquippedItem).RateOfFire = 1 end, reload, then hold down fire, your bg gun will fire freely. If you let go of fire and then try again it'll get stuck by the low ROF on the FG gun.
Anyway, this is a particular and silly edge case and probably not worth dealing with, but I ran into it so I figured I should mention it.
Entities/HeldDevice.cpp
Outdated
| // Note - to avoid item HUDs flickering in and out, we need to add a little leeway when hiding them if they're already displayed. | ||
| if (m_SeenByPlayer.at(viewingPlayer) && unheldItemDisplayRange > 0) { unheldItemDisplayRange += 3.0F; } | ||
| m_SeenByPlayer.at(viewingPlayer) = unheldItemDisplayRange < 0 || (unheldItemDisplayRange > 0 && g_SceneMan.ShortestDistance(m_Pos, g_CameraMan.GetScrollTarget(whichScreen), g_SceneMan.SceneWrapsX()).MagnitudeIsLessThan(unheldItemDisplayRange)); | ||
| if (!m_SeenByPlayer.at(viewingPlayer)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code seems kinda overcomplicated - you have 2 separate lines of code doing basically the same thing for m_SeenByPlayer.
Why not just make it so if vel magnitude is > 2.0, you break out of this more cleanly? I know it's slightly different than your intent of making it so weapons don't show labels for a bit when they're dropped, but it's probably fine for weapons that are flying through the air to now show labels anyway.
Alternatively, you instead of this wonky method of doing things, you could have some separate m_CanShowLabel flag that is set when the HD collides with terrain and unset when it's attached to something, and respect it in here. That'd be the most correct way to do things I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried for the longest time to figure out a solution that didn't require a new property flag lol
If I had to, I would use a timer for the purpose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just make it so if vel magnitude is > 2.0, you break out of this more cleanly? I know it's slightly different than your intent of making it so weapons don't show labels for a bit when they're dropped, but it's probably fine for weapons that are flying through the air to now show labels anyway.
I guess you're not a fan of this option?
And I get not wanting to make a new property but in general I don't think you need to be worried about that, it's not a big deal (especially since this one doesn't require ini or lua bindings) and it'd be a lot cleaner than it is now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ultimately okay leaving this as-is though, I guess. I don't wanna deal with it haha
…ounds position Const ptr -> ptr to const Misc cleanup
…r precision) Made buy menu descriptions use new rounding mode and round to 1 sigfig if mass is < 50. This is a bit arbitrary, but it makes it so you generally get no decimal places on bodies but do get em on guns
MovableMan::ChangeActorTeamnow calls a function that forces player to lose control of an actor that's being teamswappedCompactingHeightthat allows customization of scrap column removalAEmitterFlash now flipped properly, instead of being hackily rotated like in the stone age