Player set/get cannotMove #2553

rookgaard · 2019-03-25T22:13:50Z

Hello. I would like to present another nice functionality which went lost in 0.X series - possibility to freeze/unfreeze player. It's nice to have feature which can be used in many quests or systems like transport by wagons.

Fixes #775

src/creature.cpp

nekiro · 2019-03-26T00:44:46Z

Please don’t use _variables. Also this is really bad, because if you freeze player and he will continously spam movement it will kick him, because he will exceed the packet per second limit. (Happens because of sendCancelWalk)

rookgaard · 2019-03-26T07:25:46Z

@nekiro I've removed _ prefix. Anyway, what is difference between

		void setNoMove(bool _canNotMove)
		{
			cannotMove = _canNotMove;
			cancelNextWalk = true;
		}

and

		void setNoMove(bool canNotMove)
		{
			cannotMove = canNotMove;
			cancelNextWalk = true;
		}

if it's only function variable? Plus, what does this have to do with the connection limits if they apply only to packets sent by player not by server? Isn't the same situation when the player is standing at the door of the house to which he can not enter and keeps the arrow down while still receiving the message "You are not invited."?

nekiro · 2019-03-26T15:10:19Z

Did you check if that happens before you wrote message? I'm convinced it will.

rookgaard · 2019-03-27T08:15:37Z

@nekiro Nope, but I removed _ as you suggested. I wouldn't call myself a C++ programmer, so some of my questions may be ridiculous.

DSpeichert · 2019-03-28T00:24:15Z

src/creature.h

@@ -408,6 +408,14 @@ class Creature : virtual public Thing
 		void setUseDefense(bool useDefense) {
 			canUseDefense = useDefense;
 		}
+		void setNoMove(bool canNotMove)


canNotMove isn't correct, cannot is a single word.

by the way, I can't find cannotMove in creature.h like you said is there.

I meant field bool cannotMove = false;

Anyway, I've changed again function naming. How would you rate changes now?

ranisalt · 2019-04-02T16:36:30Z

I'd call the variable canMove, default to true and call every method get/set CanMove instead, for the sake of consistency.

rookgaard · 2019-04-02T19:50:34Z

Ok @ranisalt, I've changed to set/get, how would you rate changes now?

ranisalt · 2019-04-02T20:34:06Z

LGTM, though I'd like to hear more from @nekiro point.

nekiro

I would really like to see how it behaves in-game, could you make a gif spamming move, when you are set to cannot move?

nekiro · 2019-04-03T01:23:14Z

src/creature.cpp

@@ -249,6 +249,11 @@ bool Creature::getNextStep(Direction& dir, uint32_t&)

 void Creature::startAutoWalk(const std::forward_list<Direction>& listDir)
 {
+	if (getPlayer() && !getPlayer()->getCanMove()) {


Player* player = getPlayer(); if (player && player->getCanMove()) {

same with sendCancelWalk

nekiro · 2019-04-03T01:25:10Z

src/creature.h

+		void setCanMove(bool move)
+		{
+			canMove = move;
+			cancelNextWalk = true;


if we set canMove to true, do we still cancelNextWalk?

I too was wondering about cancelNextWalk always being set to true in this function. What is the reason for the code not being this or similar

Suggested change

cancelNextWalk = true;

cancelNextWalk = !move;

I'm still curious about this.

My guess is to fix desync issues mid-action, this is called to toggle the states, but no matter which direction it toggles to, you dont want to give the user half a sqm headstart as it may create odd client behavior. So even if you just got permission to walk, the character pos gets a cancel byte one last time to set the timings straight. (I might be wrong though).

@Znote but if player is already movement blocked then he cannot walk, so what desynchro?

EPuncker · 2019-09-06T22:12:04Z

I would really like to see how it behaves in-game, could you make a gif spamming move, when you are set to cannot move?

I've just tested it and indeed it kicks the player because of the maxPacketsPerSecond setting

DSpeichert · 2019-10-13T04:38:25Z

Should a player be exempt from maxPacketsPerSecond kick when they are not able to walk?

nekiro · 2019-10-13T09:12:58Z

Should a player be exempt from maxPacketsPerSecond kick when they are not able to walk?

No, that shouldnt be done fully server side in the first place.

DSpeichert · 2019-10-13T14:36:03Z

We can't really do it client-side, so are we merging it as-is or adding some workaround?

EPuncker · 2019-11-24T22:23:30Z

closes #775 if merged

nekiro · 2019-11-24T22:35:34Z

Do we even need it? I dont think thats something that should be in tfs repo as its mainly client sided.

rookgaard · 2019-11-25T12:19:00Z

Many servers still use the Cipsoft client to play, so due to the fact that it is quite difficult to edit, in my opinion it is easier to "block" it server-side.

edit: @EPuncker I've checked, that keeping arrow pushed it spams packets exactly the same way when player tries to enter house where's not invited to.

rookgaard · 2020-01-04T08:37:45Z

@DSpeichert Is there anything else I need to change in my request?

DSpeichert

Looking at canMove and setMove, I think it would be better to use terms such as:
isMovementBlocked: true/false
setMovementBlocked: true/false

That, IMHO, is less ambiguous.

DSpeichert · 2020-01-18T00:09:05Z

src/creature.h

+		void setCanMove(bool move)
+		{
+			canMove = move;
+			cancelNextWalk = true;


I'm still curious about this.

rookgaard · 2020-06-07T23:06:31Z

@DSpeichert @nekiro Can you look at it in free time? I've committed changes you requested.

src/creature.h

src/luascript.cpp

EPuncker · 2020-10-29T08:42:51Z

I hope we could get this merged soon, from time to time people are always requesting this function on the forum, other than the two minor code styling that I requested, everything else seems fine, I'm using this in my test server for some time already

Znote

Pending patch to PR @rookgaard has to merge: (in his repo) rookgaard#1

Znote · 2020-11-05T20:04:34Z

src/creature.h

+		void setCanMove(bool move)
+		{
+			canMove = move;
+			cancelNextWalk = true;


My guess is to fix desync issues mid-action, this is called to toggle the states, but no matter which direction it toggles to, you dont want to give the user half a sqm headstart as it may create odd client behavior. So even if you just got permission to walk, the character pos gets a cancel byte one last time to set the timings straight. (I might be wrong though).

Code style formatting fixes

Znote · 2020-11-08T09:53:40Z

Do we even need it? I dont think thats something that should be in tfs repo as its mainly client sided.

Absolutely, lots of people has requested this. I also remember being rather desperate for someone to make something like this back in the day when I was hosting servers. This has been a frustration solved by uglier hacks all the time over the past 10 years.

outdated

rookgaard · 2020-11-20T11:33:56Z

Is there anything I can do more on this PR @DSpeichert ?

EPuncker

to me there is just a small request, the addition of the entries in compat.lua for it to be compatible with the old version:

getCreatureNoMove(cid)
doCreatureSetNoMove(cid, block)

rookgaard · 2020-11-29T22:28:41Z

@EPuncker I've added functions to compat.lua.

lua & game functions

a3f091b

DSpeichert requested changes Mar 25, 2019

View reviewed changes

src/creature.cpp Outdated Show resolved Hide resolved

code review changes

30d33c3

compiler fix

e7cb0ba

DSpeichert reviewed Mar 28, 2019

View reviewed changes

changed function name

7af516a

Znote previously approved these changes Apr 1, 2019

View reviewed changes

another renaming

507b871

nekiro requested changes Apr 3, 2019

View reviewed changes

DSpeichert previously requested changes Jan 18, 2020

View reviewed changes

src/creature.h

void setCanMove(bool move)

{

canMove = move;

cancelNextWalk = true;

Copy link

Member

DSpeichert Jan 18, 2020

Choose a reason for hiding this comment

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

I'm still curious about this.

This comment was marked as abuse.

Sign in to view

rookgaard added 2 commits June 8, 2020 00:52

Merge branch 'master' into feature/cannotMove

6080fad

CR changes

15cc06f

rookgaard dismissed Znote’s stale review via 15cc06f June 7, 2020 23:05

rookgaard requested a review from DSpeichert June 7, 2020 23:05

rookgaard requested a review from nekiro June 7, 2020 23:06

DSpeichert requested a review from a team June 22, 2020 07:01

EPuncker requested changes Jun 23, 2020

View reviewed changes

src/creature.h Outdated Show resolved Hide resolved

src/luascript.cpp Outdated Show resolved Hide resolved

EPuncker requested a review from Znote October 29, 2020 08:43

Code style formatting fixes

ec1be5f

Znote reviewed Nov 5, 2020

View reviewed changes

rookgaard and others added 2 commits November 6, 2020 00:26

Merge branch 'master' into feature/cannotMove

d4e874e

Merge pull request #1 from Znote/cannotMove

1823e63

Code style formatting fixes

Znote previously approved these changes Nov 6, 2020

View reviewed changes

Znote requested a review from EPuncker November 8, 2020 09:47

EPuncker requested changes Nov 20, 2020

View reviewed changes

DSpeichert added the feature New feature or functionality label Nov 29, 2020

rookgaard added 2 commits November 29, 2020 23:23

Merge branch 'master' into feature/cannotMove

dee988c

added compat functions

c77efb5

rookgaard dismissed Znote’s stale review via c77efb5 November 29, 2020 22:27

EPuncker approved these changes Nov 30, 2020

View reviewed changes

Znote approved these changes Nov 30, 2020

View reviewed changes

DSpeichert merged commit 3b42c0e into otland:master Dec 1, 2020

Zbizu mentioned this pull request Dec 2, 2020

onMove and onDropLoot #1049

Closed

rookgaard deleted the feature/cannotMove branch March 17, 2021 06:02

Player set/get cannotMove #2553

Player set/get cannotMove #2553

Conversation

rookgaard commented Mar 25, 2019 • edited by DSpeichert

nekiro commented Mar 26, 2019

rookgaard commented Mar 26, 2019 • edited

nekiro commented Mar 26, 2019 • edited

rookgaard commented Mar 27, 2019

DSpeichert Mar 28, 2019

Choose a reason for hiding this comment

rookgaard Mar 28, 2019

Choose a reason for hiding this comment

ranisalt commented Apr 2, 2019

rookgaard commented Apr 2, 2019

ranisalt commented Apr 2, 2019

nekiro left a comment

Choose a reason for hiding this comment

nekiro Apr 3, 2019

Choose a reason for hiding this comment

nekiro Apr 3, 2019

Choose a reason for hiding this comment

soul4soul Apr 3, 2019 • edited

Choose a reason for hiding this comment

DSpeichert Jan 18, 2020

Choose a reason for hiding this comment

Znote Nov 5, 2020 • edited

Choose a reason for hiding this comment

nekiro Nov 8, 2020

Choose a reason for hiding this comment

EPuncker commented Sep 6, 2019

DSpeichert commented Oct 13, 2019

nekiro commented Oct 13, 2019

DSpeichert commented Oct 13, 2019

EPuncker commented Nov 24, 2019

nekiro commented Nov 24, 2019

rookgaard commented Nov 25, 2019 • edited

rookgaard commented Jan 4, 2020

DSpeichert left a comment

Choose a reason for hiding this comment

DSpeichert Jan 18, 2020

Choose a reason for hiding this comment

This comment was marked as abuse.

rookgaard commented Jun 7, 2020

EPuncker commented Oct 29, 2020 • edited

Znote left a comment

Choose a reason for hiding this comment

Znote Nov 5, 2020 • edited

Choose a reason for hiding this comment

Znote commented Nov 8, 2020

rookgaard commented Nov 20, 2020

EPuncker left a comment

Choose a reason for hiding this comment

rookgaard commented Nov 29, 2020

rookgaard commented Mar 25, 2019 •

edited by DSpeichert

rookgaard commented Mar 26, 2019 •

edited

nekiro commented Mar 26, 2019 •

edited

soul4soul Apr 3, 2019 •

edited

Znote Nov 5, 2020 •

edited

rookgaard commented Nov 25, 2019 •

edited

EPuncker commented Oct 29, 2020 •

edited

Znote Nov 5, 2020 •

edited