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

Adds support for weapons with reach #24395

Merged
merged 5 commits into from Mar 11, 2017
Merged

Adds support for weapons with reach #24395

merged 5 commits into from Mar 11, 2017

Conversation

ghost
Copy link

@ghost ghost commented Feb 24, 2017

As requested by like three people on the forums. Want a whip that can smack people from long-distance? Now you can. This PR doesn't change the reach of any weapons but just adds support, so we can hopefully make combat more interesting.

@ChangelingRain
Copy link
Contributor

This bypasses obstacles. You need a bit of tuning, mayhap.

@ghost
Copy link
Author

ghost commented Feb 24, 2017

Any recommendations? Don't know how to find dense objects in a path

@PKPenguin321
Copy link
Contributor

Hey somebody's actually coding it!!! ❤ xhuis the god

@PKPenguin321
Copy link
Contributor

Oh this will need support for recognizing the distance hit from so we can have different effects for hits at different ranges

@lzimann lzimann added the Feature Exposes new bugs in interesting ways label Feb 24, 2017
@@ -129,7 +129,7 @@

// Allows you to click on a box's contents, if that box is on the ground, but no deeper than that
if(isturf(A) || isturf(A.loc) || (A.loc && isturf(A.loc.loc)))
if(A.Adjacent(src)) // see adjacent.dm
if(A.Adjacent(src) || W.reach >= get_dist(src, A)) // see adjacent.dm
Copy link
Contributor

Choose a reason for hiding this comment

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

What if someone wants to make a weapon that only works when standing on the same tile?

Copy link
Author

Choose a reason for hiding this comment

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

Not supported unfortunately unless I remove the adjacent check. That's not something I'd do lightly, either!

Choose a reason for hiding this comment

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

else if(!A.Adjacent(src)) && w.reach blah blah can do the trick, no?

Copy link
Author

Choose a reason for hiding this comment

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

I suppose it could.

Copy link
Contributor

Choose a reason for hiding this comment

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

A weapon that only works on the same tile already works.
Look at the logic, it's Adjacent OR having a reach more or equal to the distance between you and the target.
if you're on top of eachother, there's no distance, so a reach of 0 would be "same tile only"

Copy link
Author

Choose a reason for hiding this comment

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

And that it would succeed regardless and make reach 0 weapons irrelevant.
Sorry for double post, goddamn phone

Copy link
Contributor

Choose a reason for hiding this comment

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

It would, but this if is an OR (||)

var/A = FALSE
var/B = TRUE
if (A || B)
    world << "wew"

this, not surprisingly, produces "wew"

Are you tired? drunk? high? I don't see how at this point in time you don't know how an OR works, if I have time at some point you'll have to throw some questions my way because these past few PRs have shown you seem really far behind.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I do see however that adjacency would always win the 0 case, if that's what you were trying to say. (The if will always succeed on a same-tile 0 range weapon)
You could snowflake a reach of 1 to be the only case that runs Adjacent(), if you -really- want 0 reach weapons.

if(W.reach == 1 && A.Adjacent(src))
    ...
else if(W.reach >= get_dist(src, A))
    if(do_some_checks_like_dense_lines_and_stuff())
        ...

Copy link
Author

Choose a reason for hiding this comment

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

Well, I woke up thirty minutes ago, so I guess I'm just tired. Here's how I'm reading it:

Adjacent is true if they're nearby.
Reach is true if they're within a certain range

Say we have the prank gun, which works on only your tile. And you have a mob on your tile that you want to prank.

Both checks pass. Mob gets pranked. YouTube ad revenue goes through the roof.

Now the mob moves one tile above you.

The reach check fails, so we check Adjacent. That one succeeds, because the mob is indeed adjacent.

Thus, the hit succeeds, because it uses either check and not both checks.

Analogies aside, it's super easy to change this, and I likely will later today to support these kind of weapons.

Copy link
Author

Choose a reason for hiding this comment

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

Right, didn't see your comment. thanks mobile git!

@RemieRichards
Copy link
Contributor

@PKPenguin321 I did this before, with full support for larger attack animation too (because otherwise you have a weapon that reaches 3 tiles but the animation only goes one turf), I didn't PR it because it just generally doesn't feel right.

and for that, I guess I'm 👎 on this

@FantasticFwoosh
Copy link
Contributor

@Xhuis why not have it just hit obstacles in its path? Then we could have something going where you could accidentally recoil yourself if you miss. Any weapon can accidentally bash pipes in maint etc.

If not a mob, add recoil (unless there is none) in not very technical terms.

@ghost
Copy link
Author

ghost commented Feb 24, 2017

It certainly can, but I'm still not sure how to get a list of turfs between here and there to check for those obstacles at. I'll have to experiment around later.

@RemieRichards
Copy link
Contributor

@Xhuis getline(A,B) will list all of the turfs between A and B, you can loop over this list to add all of those turf's contents to another list, and go through everything.

alternatively, some kind of projectile.

@ghost
Copy link
Author

ghost commented Feb 24, 2017

That solves most of my problems. Thanks! I still don't know if this is worth it, though, because of how much slower it's going to make every attack.

@RemieRichards
Copy link
Contributor

@Xhuis simple, only run that code IF it's the reach that caused the attack, if it was adjacency that caused it, there's no real line to check anyway.

@ghost
Copy link
Author

ghost commented Feb 24, 2017

At this point, I have a dilemma. Either I can cut out the adjacent check entirely and replace it with the reach check and an incapacitated check to stop silly attacks, or I can put in reach checks for special cases. Which one would be better?

@RemieRichards
Copy link
Contributor

RemieRichards commented Feb 24, 2017

Well, ideally it'd all be one proc that did the "is this attack valid?", so you'd need a proc that uses getline(), and also does all the checks Adjacent() does.

the getline() is apparently a superfast algorithm, so running it for 1-tile distances probably won't be any more costly than Adjacent()

@ghost
Copy link
Author

ghost commented Feb 24, 2017

Hm. So, like, a new proc called 'CheckReach' or something? That could work, especially if I just include that adjacency check in it for weapons of range 1 or higher.

I'll have to do that when I get home, though, because I'm not a computer whiz and I won't write code from my phone on the GitHub editor.

@RemieRichards
Copy link
Contributor

RemieRichards commented Feb 24, 2017

@Xhuis yes, exactly, CheckReach() sounds like an appropriate name also.
One way of doing it, perhaps, is:

/proc/CheckReach(atom/A, atom/B, reachDist = 1)
    var/list/turfs = getline(A,B)
    switch(turfs.len)
        if(1) //same turf
            return (reachDist >= 0)
        if(2) //might need to check this, but for adjacent getline() probably returns 2
            return A.Adjacent(B)
        if(3 to INFINITY)
            var/turf/S = turfs[1]
            var/turf/E = turfs[2]
            var/distTravelled = 1
            while(S.Adjacent(E) && (distTravelled <= reachDist) && (distTravelled <= turfs.len))
                S = E
                E = turfs[distTravelled+2]
                distTravelled++
            return (E == get_turf(B))
    return FALSE

...
if(CheckReach(src, A, W.reach))
    ...

Not tested, but that should be p.solid, it's basically:
List(AT, 2, 3, 4, 5, 6, 7, 8, 9, BT)
AT.Adj(2) -> 2.Adj(3) -> 3.Adj(4) -> 4.Adj(5) -> 5.Adj(6)... BT-1.Adj(BT)

@ghost
Copy link
Author

ghost commented Feb 24, 2017

Sounds good to me. Any way to check if a dense object actually blocks an attack, I.e. a table won't but a window can?

@RemieRichards
Copy link
Contributor

@Xhuis Adjacent() calls ClickCross(target_dir, border_only, target_atom = null), which checks the turf for dense objects and things.

@ghost
Copy link
Author

ghost commented Feb 24, 2017

Well, that solves a lot. Adjacent it is!

@RemieRichards
Copy link
Contributor

@Xhuis you mean CheckReach() yeah? :^) (since its actual main check-y part is itself just an Adjacent() call)

@ghost
Copy link
Author

ghost commented Feb 24, 2017

you know that that's what I meant >:(

@imtakingabreakdontatme
Copy link
Contributor

I dont think there is any good way to communicate reach weapons visually

Or to even balance them for that matter

@PKPenguin321
Copy link
Contributor

They would probably need large inhands (which I think we support, remie added it I think?) and to draw a beam from attacker to victim. They could definitely be balanced by making different hit effects depending on how far away you hit a target from. Check the thread on the forums

@ghost
Copy link
Author

ghost commented Feb 27, 2017

Just a small update so you know that I haven't abandoned this and close-happy maintainers don't shut it down for being stale. I haven't worked much in this, as I'm still not sure if the niche cases that this will apply to is worth checking every attack for. I also still don't know how to check if something should block an attack. Density and opacity, the two variables I could use, both don't make sense, and I don't know if adding an obstruction variable to every atom would be wise.

@ghost
Copy link
Author

ghost commented Feb 27, 2017

Can anyone think of some cases where this framework might actually be used? Spears are all that I can think of, making me apprehensive about even coding this further.

@Militaires
Copy link

long dongs that reach across zlevels when

@PKPenguin321
Copy link
Contributor

Xhuis, go check the forum thread again

GIVE librarian whip

@ghost
Copy link
Author

ghost commented Mar 1, 2017

Doing some more work on this today. I can't go with @RemieRichards' proc as-is, because it doesn't account for dense objects that might also logically allow an attack to go past them (such as a rack or a table). To resolve this, I'm adding a new var to /atom, obstructs, to check if it blocks attacks like that. Each turf checked by CheckReach will have objects in them checked for obstructs.

If you know a better way for this, I'd be happy to take it.

@ChangelingRain
Copy link
Contributor

honestly just make reach weapons.... basically guns, except with instant projectiles and a longer attack cooldown

@ghost
Copy link
Author

ghost commented Mar 1, 2017

That's... an odd way of doing it, but not a bad one either. Projectiles already check this kind of thing, too, so I suppose I could refer to their code for a more efficient way of doing this.

@optimumtact optimumtact added the Stale Even the uncaring universe rejects you, why even go on label Mar 2, 2017
@ghost
Copy link
Author

ghost commented Mar 3, 2017

The argument I'm currently having with Remie reminded me of this, so I'll continue on it once I get home.

@ghost
Copy link
Author

ghost commented Mar 3, 2017

All right, done! I added proper checking through dense objects. The method I ended up using was what @ChangelingRain recommended; I used an invisible dummy object that moved from turf to turf, checking if it could pass by. The attacks pass over the same things that projectiles do.

@PKPenguin321
Copy link
Contributor

PKPenguin321 commented Mar 3, 2017

Did you ever do this

Oh this will need support for recognizing the distance hit from so we can have different effects for hits at different ranges

I cant tell if your code does that

@ghost
Copy link
Author

ghost commented Mar 3, 2017

No, but it's very easy to implement with a quick get_dist(here, there) check. You'd need to modify weapon code, too.

@ghost
Copy link
Author

ghost commented Mar 4, 2017

This should be ready.

return 1
if(!dummy.Move(T)) //we're blocked!
return
return
Copy link
Contributor

Choose a reason for hiding this comment

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

aaaaaaaa training return

if(2 to INFINITY)
var/obj/dummy = new(get_turf(here)) //We'll try to move this every tick, failing if we can't
dummy.pass_flags |= PASSTABLE
QDEL_IN(dummy, 10) //1-second grace period for calculation
Copy link
Contributor

Choose a reason for hiding this comment

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

just qdel it after the loop, no need to keep it around

@optimumtact optimumtact added Ready for Review and removed Stale Even the uncaring universe rejects you, why even go on labels Mar 7, 2017
@optimumtact optimumtact merged commit 7db2a60 into tgstation:master Mar 11, 2017
@ghost ghost deleted the deus_vult branch March 11, 2017 05:09
@PKPenguin321
Copy link
Contributor

yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Exposes new bugs in interesting ways
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants