ID:2484803
 
(See the best response by Ter13.)
So I am currently trying to make a system so that the player can drag other players, objects and mobs around.
So far ive got it to work, except it only works whilst going east or north.
mob
Move()
if(src.ingame ==1)
if(src.pulling)
for(var/atom/movable/O in world)
if(O.pulledby == src)
O.Move(last_loc)
if(src.isNPC == 1)
return ..()
if(src.frozen)
return 0
if(!src.looking == 0)
src.looking=0
src.last_loc = src.loc
return ..()
for(var/atom/movable/O in world)


We meet again...

The code you've posted doesn't explain why it's going only north or East. Is move defined elsewhere as well?
Ah hiya, no it is not. Altough i am using a pixel movement script to allow for proper diagonal travel.
client
New()
SetMacros()
..()
var
step_dir = 0 // Direction the user is facing
verb
KeyDown(n as num)
set hidden = 1
step_dir += n
walk(src.mob, step_dir)
if(n == 1)
usr.IsPressingW = 1
if(n == 2)
usr.IsPressingS = 1
if(n == 8)
usr.IsPressingA = 1
if(n == 4)
usr.IsPressingD = 1
if (usr.IsMoving == 1)
return
else
if(usr.unconscious == 1)
return
else
usr.IsMoving = 1
mob.StepSound()
KeyUp(n as num)
set hidden = 1
step_dir -= n
if(step_dir <= 0)
step_dir = 0
walk(src.mob,null)
else
walk(src.mob, step_dir)
if(n == 1)
usr.IsPressingW = 0
if(n == 2)
usr.IsPressingS = 0
if(n == 8)
usr.IsPressingA = 0
if(n == 4)
usr.IsPressingD = 0
if(usr.IsPressingA == 0)
if(usr.IsPressingD == 0)
if(usr.IsPressingW ==0)
if (usr.IsPressingS == 0)
usr.IsMoving = 0

// Set the macros to be used
SetMacros()
set hidden = 1
var/list/directions = list(
"w" = 1,
"s" = 2,
"a" = 8,
"d" = 4,
"North" = 1,
"South" = 2,
"West" = 8,
"East" = 4
) //List of directions to be used.

for(var/i in directions)
var/d = directions[i]
winset(src, "[i]macro", "parent=macro;name=[i];command=[url_encode("KeyDown [d]")]")
winset(src, "[i]macroUp", "parent=macro;name=[i]+UP;command=[url_encode("KeyUp [d]")]")
Best response
Right, so this is a major logic problem.

You are calling Move() more often than the player is allowed to move, and since you are setting last_loc every time the player tries to move, the loc is very often going to be the player's current loc.

The other issue is that you are trying to move the object the player is pulling before checking if the player actually moved.

Let's address a few other issues first:

1) for in world

This is the worst possible way to do anything that requires you to act on specific objects. The only case where you want to hunt the entire world for specific objects is when the worst possible way is also the best possible way.

                for(var/atom/movable/O in world)
if(O.pulledby == src)
O.Move(last_loc)


Let's replace this with a better solution:

atom
movable
var/tmp
mob/dragger

mob
var/tmp
list/dragging
proc
GrabObject(atom/movable/o)
o.dragger?.ReleaseObject(o)
if(!dragging)
dragging = list()
dragging += o
o.dragger = src

ReleaseObject(atom/movable/o)
o.dragger = null
dragging -= o
if(!dragging.len)
dragging = null

ClearDragging()
for(var/atom/movable/o in dragging)
o.dragger = null
dragging = null

Del()
ClearDragging()
..()



Call GrabObject to start dragging something. Call ReleaseObject to stop dragging something. ClearDragging whenever you don't want a player to keep dragging anything that they are currently dragging.


Right, so now that we are storing the objects the player is dragging, we don't have to look through the entire world to check what they are dragging. We only look through a list only containing the objects that the player is dragging. This saves you significant CPU in the long run. Looping through the world like that has to search everything in the world, and it's slow. It may "work fine" and it may be easy, but eventually, writing code like this will cause your project to shit the bed.


2) We actually shouldn't even bother storing last_loc. When Move() starts, we can store the player's current location, and then call the default action to try to move. If the move is successful, we can move the objects to the stored location. No need to store it in a src variable between calls, because the information will become unreliable.

mob
Move()
if(src.frozen)
return 0
if(!src.looking == 0)
src.looking=0
var/oldloc = loc
. = ..()
if(.)
for(var/atom/movable/o in dragging)
o.Move(oldloc)
Okay so I do like to work out how code works before i Put it in.
I think I've got the gist of how your code is supposed to work but I feel i am missing something, its in practically as you have written, Ive added a verb to the mob/obj i want to drag but it does nothing after clicking grabobject.
mob
mobtodrag
verb
Drag()
set src in oview(0.5)
GrabObject()
Release()
set src in oview(0.5)
ReleaseObject()

Could you kindly explain what the
.=..() means?
oview(0.5) is not valid. oview() takes a tile range. There is no such thing as a half tile.

. is a variable for the current return value of a proc.

..() is the supercall, or call the behavior the current proc is overriding.

. = ..() means set the current return value to the return value of the old behavior we overrode, but do not return yet.

mob
mobtodrag
verb
Drag()
set src in oview(0.5)
GrabObject()
Release()
set src in oview(0.5)
ReleaseObject()


You are using GrabObject and ReleaseObject wrong, you also have this defined on the wrong object. GrabObject and ReleaseObject() are called on the dragger with the argument being the dragged object.

This is what you wanted to have been doing:

atom/movable
verb
Drag()
set src in oview(1)
usr.GrabObject(src)
Release()
set src in oview(1)
usr.ReleaseObject(src)
I mean, oview(0.5) works fine on absolutely everything else in the game? Also so putting .=..() basically says "Now continue the original proc then come back to this after"?

Also it is kinda working now, but the dragged mob goes absolutely loopy.
https://www.youtube.com/watch?v=jqlKJ4veE-E&feature=youtu.be


EDIT
okay with some testing it seems pixel movement is causing the issue. If i turn off all boundaries and set step size to 64 and lower the fps it works perfectly.
Okay, well yeah, the way you were using Move() was the tile movement format. You weren't using pixel offsets, so your initial code was wrong for pixel movement anyway.
Right okay then. This is gonna be great to work out. What would my options be then? Im a little confused with how the base pixel movement works.
Move() in pixel movement mode does not necessarily complete a step. It can be stopped at any point along the desired path of the movement. So you are going to need to work out how far the player actually moved, and then move the objects being dragged in that direction for that distance at the very minimum.

This may not produce the desired behavior, however, as you may actually want the object to try to draft behind the player. Let's solve the first solution first though because it's the easiest:

Note that I am going to neglect anything that is not exactly related to solving this problem. Any other code that is related to any other part of your game is going to be left out.

//define TILE_WIDTH/TILE_HEIGHT to match your world's tile width and height. We need this for the below code to work at all. So don't mess this up:

#define TILE_WIDTH 32
#define TILE_HEIGHT 32

//build a datum to help us convert between global pixel and loc/step based coordinates:

storedloc
var
atom/loc
step_x = 0
step_y = 0

New(loc,step_x,step_y)
src.loc = loc
src.step_x = step_x
src.step_y = step_y

proc
Convert(gx,gy,z)
loc = locate(gx/TILE_WIDTH,gy/TILE_HEIGHT,z)
if(loc)
step_x = gx%TILE_WIDTH
step_y = gy%TILE_HEIGHT
else
step_x = 0
step_y = 0

Ref(atom/ref)
loc = ref.loc
if(istype(ref,/atom/movable))
step_x = ref:step_x + ref:bound_x
step_y = ref:step_y + ref:bound_y
else
step_x = 0
step_y = 0

//now we'll handle moving the objects identically to how the player moves.
mob
Move()
//store old location data before the movement
var/turf/oldloc
var/opx = step_x + bound_x
var/opy = step_y + bound_y
. = ..()
if(.)
//calculate the distance actually moved during the successful movement
var/ox = (x*TILE_WIDTH + step_x + bound_x) - (oldloc.x * TILE_WIDTH + opx)
var/oy = (y*TILE_HEIGHT + step_y + bound_y) - (oldloc.y * TILE_HEIGHT + opy)

var/storedloc/sloc = new/storedloc()
var/gx, gy
for(var/atom/movable/o in dragging)
//calculate the offset position for each object you are dragging
gx = o.x*TILE_WIDTH + o.step_x + o.bound_x + ox
gy = o.y*TILE_HEIGHT + o.step_y + o.bound_y + oy

//convert the global coordinates to step-based coordinates
sloc.Convert(gx,gy,o.z)
o.Move(sloc.loc,o.dir,sloc.step_x,sloc.step_y)
So one of the issues with the above approach is that it does not do what you probably want.

What we should really be doing, is a bit more involved than this. We need to determine if the object is touching the player's bounding box. If so, do not move. If not, we need to move the object until the object's bounding box is touching the edge of the player's bounding box, but we can't just move to a touching position. We need to move toward the center of the player's bounding box from the center of the object's bounding box.

Which, honestly, is something I really don't want to type out in a developer help thread. It's beyond the scope of what I'm willing to do for free, and it's something well beyond the scope of what you are capable of doing yourself right now. It wouldn't help you at this time for me to write this code for you.
Alright, well thank you for your help anyway, it has been greatly appreciated.
Good grief,
Got it working to a fair degree using step_to
instead.
Haha
Would suggest fiddling around with your code more often and observing the results.