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

[no squash] Send ActiveObjects along with definitions once connection is established #8834

Merged
merged 5 commits into from Sep 14, 2019

Conversation

@ClobberXD
Copy link
Contributor

commented Aug 21, 2019

  • ActiveObjects are also sent once along with itemdefs and nodedefs, once the client reaches CS_InitDone.
    • This means that active objects are sent to the client earlier than in master, which sends activeobjects after all definitions are sent (the next step). While this isn't much of a difference, it becomes more noticeable the more the client is lagging.
    • This required some refactoring in server.cpp to prevent substantial code duplication.
  • LocalPlayer::isDead always returned false if damage had been disabled, and that is now fixed - The method checks whether the player's CAO is immortal, and then whether the CAO's HP is 0.
  • Lots of code-style fixes in localplayer.cpp and Client::sendPlayerPos (in client.cpp), including
    • Pointer placement
    • Spacing between keywords, parenthesis and braces. e.g. if ( and ) {
    • Spacing around operators in expressions
    • Merging braces and else keyword into one line. i.e. } else {
    • Removal of double-tab indents where not required.
    • Addition of missing literal f for floats.
@ClobberXD ClobberXD force-pushed the ClobberXD:fix_isDead_when_immortal branch from f7b8c8c to 6b25d53 Aug 25, 2019
@ClobberXD ClobberXD changed the title Fix LocalPlayer::isDead always returning false if player is immortal Send ActiveObjects along with definitions once connection is established Aug 26, 2019
@ClobberXD ClobberXD changed the title Send ActiveObjects along with definitions once connection is established [no squash again, sorry] Send ActiveObjects along with definitions once connection is established Aug 26, 2019
@ClobberXD ClobberXD changed the title [no squash again, sorry] Send ActiveObjects along with definitions once connection is established [no squash] Send ActiveObjects along with definitions once connection is established Aug 26, 2019
@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

This PR is ready for review!

@SmallJoker I haven't removed LocalPlayer::hp in this PR, as this PR is quite large already, and replacing direct accesses to this variable with getCAO()->setHP() means even more changes. I'm afraid it'll have to wait for a little more. :)

@ClobberXD ClobberXD marked this pull request as ready for review Aug 26, 2019
@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

This PR is ready for review!

@SmallJoker I haven't removed LocalPlayer::hp in this PR, as this PR is quite large already, and replacing direct accesses to this variable with getCAO()->setHP() means even more changes. I'm afraid it'll have to wait for a little more. :)

@ClobberXD ClobberXD force-pushed the ClobberXD:fix_isDead_when_immortal branch 3 times, most recently from f72331a to 3104c01 Aug 26, 2019
src/client/localplayer.cpp Outdated Show resolved Hide resolved
src/client/localplayer.cpp Outdated Show resolved Hide resolved
src/client/localplayer.cpp Outdated Show resolved Hide resolved
src/client/localplayer.cpp Outdated Show resolved Hide resolved
src/client/localplayer.cpp Show resolved Hide resolved
src/client/localplayer.cpp Outdated Show resolved Hide resolved
src/client/localplayer.cpp Outdated Show resolved Hide resolved
src/network/serverpackethandler.cpp Outdated Show resolved Hide resolved
src/server.cpp Outdated Show resolved Hide resolved
@ClobberXD ClobberXD force-pushed the ClobberXD:fix_isDead_when_immortal branch 3 times, most recently from 08e8ac2 to 189b20d Aug 27, 2019
@ClobberXD ClobberXD force-pushed the ClobberXD:fix_isDead_when_immortal branch from 189b20d to 838bc8b Aug 28, 2019
@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2019

@SmallJoker I've addressed all your line comments.

@ClobberXD ClobberXD force-pushed the ClobberXD:fix_isDead_when_immortal branch from 838bc8b to 06ccf05 Aug 28, 2019
@ClobberXD ClobberXD force-pushed the ClobberXD:fix_isDead_when_immortal branch from 06ccf05 to 79f384f Aug 31, 2019
@sfan5

This comment has been minimized.

Copy link
Member

commented Aug 31, 2019

What happens when you connect to an older (say 5.0) server? Will autoforward still be broken?

@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

The only code that potentially invokes LocalPlayer::isDead prior to AOs being sent, is Client::sendPlayerPos, which first checks if m_activeobjects_sent is true, and only then invokes that function. m_activeobjects_sent is false upon initialisation, and is set to true only after the first TOCLIENT_ACTIVEOBJECTS_REMOVE_ADD packet is received. That means LocalPlayer::isDead won't be invoked at all, if active objects haven't been received by the client.

While I've tested that m_activeobjects_sent is correctly set and that isDead isn't invoked otherwise, I haven't tested connecting to an older server. It should work, for the aforementioned reasons, but I'll test, just to be sure.

@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Sep 11, 2019

@sfan5 Connecting to an older server does not seem to cause any problems, and I can see all the loaded AOs upon connection without any issues. So this PR doesn't cause any breakages in clients when connecting to older servers.

@ClobberXD ClobberXD requested a review from SmallJoker Sep 11, 2019
@paramat paramat added this to the 5.1.0 milestone Sep 11, 2019
@paramat

This comment has been minimized.

Copy link
Member

commented Sep 11, 2019

Needed for 5.1.0.

@ClobberXD ClobberXD force-pushed the ClobberXD:fix_isDead_when_immortal branch 2 times, most recently from 6940ae2 to 657acd0 Sep 12, 2019
src/client/client.h Outdated Show resolved Hide resolved
src/server.cpp Outdated Show resolved Hide resolved
@SmallJoker

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

Works, will approve as soon the comments are handled.

ClobberXD added 5 commits Aug 25, 2019
@ClobberXD ClobberXD force-pushed the ClobberXD:fix_isDead_when_immortal branch from 657acd0 to ce441f9 Sep 13, 2019
@ClobberXD

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2019

@SmallJoker, @nerzhul - Both requested changes have been addressed, and the PR has been re-tested to ensure that it remains functionally intact.

@ClobberXD ClobberXD requested review from SmallJoker and nerzhul Sep 13, 2019
@sfan5
sfan5 approved these changes Sep 14, 2019
Copy link
Member

left a comment

Looks good.

@sfan5 sfan5 added >= Two approvals and removed One approval labels Sep 14, 2019
@sfan5 sfan5 merged commit 8e42a25 into minetest:master Sep 14, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@paramat

This comment has been minimized.

Copy link
Member

commented Sep 14, 2019

My 'W' finger thanks you.

@ClobberXD ClobberXD deleted the ClobberXD:fix_isDead_when_immortal branch Sep 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.