matpow2 / cuwo

Open server and utilities for Cube World
GNU General Public License v3.0
170 stars 49 forks source link

Updated PVP script #149

Closed UserXXX closed 10 years ago

UserXXX commented 10 years ago

As the default PVP script delivered with cuwo is very simple, I decided to extend it. Additional information:http://cubeworldforum.org/topic/15853-advanced-pvp-script-for-cuwo/

matpow2 commented 10 years ago

Hi UserXXX

You don't seem to conform to PEP8 (especially with regards to the column width). Also, you are creating objects in the class objects, not the instance objects (e.g. entity_id_mapping), which is probably not what you intend. Also, we already have the CubeWorldServer.entities dict, use that.

Are the ENTITYHOSTILITY* values properly reverse-engineered? Anticheat will most likely force the value to 0. IIRC, those values are typically reserved for mobs, and probably make other assumptions about the HP.

Do not create your own ServerUpdate objects. Use the packet in the server object, i.e. CubeWorldServer.update_packet. You are bound to break the server otherwise.

The HP workaround seems quite hacky, but I like some of the other additions - maybe I will add something equivalent for the upstream PVP script.

UserXXX commented 10 years ago

Hi Mat^2, I will have a look at these points and "correct" the issues.

That depends on the interpretation of "properly reverse engineered", some values come from this site: http://www.cubeworldwiki.net/index.php/Multiplayer_Protocol, especially the ENTITY_HOSTILITY_HOSTILE value. The ENTITY_HOSTILITY_FRIENDLY_PLAYER is used by you too and so properly reverse engineered (I think). The only value I'm not sure about is the ENTITY_HOSTILITY_FRIENDLY, but it seems as if it has the same conditions as _HOSTILE. I also tried other values to make green status bars for "neutral" entities, but the values up to 6 didn't do anything like that and had the same effect as _FRIENDLY. The HP is handled correctly (the damage arrives at the other client), but (I don't know whether it is a client bug or intended by wol_lay) it is shown wrong. I don't know whether the max_hp_multiplier * 2 is correct, but it seems so.

Didn't see, that this packet is declared in the server class, I tried to import the entity update packet from server.py, but as it then tries to install the reactor a second time that makes the server crash, so I thought I would have to handle all packets that way.

PS: Sorry for the really terrible english in the first post, I think my keyboard played me a prank, half of the sentence is missing.

matpow2 commented 10 years ago

I noticed the change of copyright as well. FYI, we generally license code under my name so we can change the license later if necessary. Names of contributors are listed in README.md though. If you wanted to add your own copyright, you would have to add another copyright line so the full line of rights are clear, i.e. something like

# Copyright (c) Bjoern Lange 2014.
# Copyright (c) Mathias Kaerlev 2013-2014.

This is just general legal advice - I don't think we would accept this into the main repository. Also, if it truly was based on the original script, legally, you can't change the license.

It seems like the code is more inspired by the PvP script than based on it, so I didn't comment on it originally.

UserXXX commented 10 years ago

While reviewing the things you mentioned in your first post I came to the point involving the entity_id_mapping. It seems that the CubeWorldServer.entities dict maps entity ids to entity data objects. The problem is that I need the reversed thing, so I would have to run through the key-value-pairs until I find the correct one. A second dict in my opinion is a better solution, but generally the question is: memory- or performance-overhead. In this case I prefer the second dict because of performance.

UserXXX commented 10 years ago

I committed the new version and would ask you to review it again.