Jump to content

Packets cleanup


xazax

Recommended Posts


  • Group:  Members
  • Topic Count:  11
  • Topics Per Day:  0.00
  • Content Count:  427
  • Reputation:   123
  • Joined:  11/17/11
  • Last Seen:  

Meyraw made a nice modification on how to handle packets.

You can check his project there: http://code.google.com/p/eathenanext/

The main point is, the packets are no longer edited with WFIFO macros, but they are structs, with members just like in Aegis.

With such system it would be easier to add support for newer clients, since we can dump those structs from Aegis leaked pdbs ( In his code as far as I remember the struct and member names does not match with aegis' ones, maybe it would worth name matching.). Adding this would be a great deal of work, breaking lot of existing patches, and probably would be better to develop it in a different branch.

Opinions?

  • Upvote 2
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  0
  • Topics Per Day:  0
  • Content Count:  32
  • Reputation:   17
  • Joined:  11/20/11
  • Last Seen:  

Generally pro. I started this about three times, but never got beyond the basic movement / chat packets. It's indeed a lot work, but worth the effort. The eathenanext approach looks fine, definitely better than the current clif.c code. I didn't read through the entire code though, just briefly skimmed it.

We currently support about six different spawn packets, possibly more by now. Structs make it easier to figure out the final packet structure, but the code remains either redundant (one function per packet in each version) or scrambled with #ifdefs (one function per semantic packet [walk, spawn, ...]).

In the process we should definitely merge packet_db versions and PACKETVER. It doesn't make any sense to support dozens of client versions, but have one fixed server protocol.

PACKET(PKT_MC_CUTIN) pkt = { PKT_MC_CUTIN };

This is weird.

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  22
  • Topics Per Day:  0.00
  • Content Count:  764
  • Reputation:   220
  • Joined:  11/14/11
  • Last Seen:  

Even though my opinion isn't relevant in this discussion I want to support this idea because this will make it easier for noobs like me to get into this part of Athena.

  • Upvote 1
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  9
  • Topics Per Day:  0.00
  • Content Count:  379
  • Reputation:   304
  • Joined:  11/10/11
  • Last Seen:  

If the cleanup (remove old packets) concern some of the packets send from client to the server, let me know !

roBrowser currently send the oldest possible packet to server to get a maximum compatibility with all pserver (since ea/ra support almost all packets send from client to server).

Well I should improve robrowser to send the packet related to the detected packetver...

Good idea through, It's more clean than the current version of clif.c.

Same thing with chrif.c and login/map-serv.

  • Upvote 2
Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  1
  • Topics Per Day:  0.00
  • Content Count:  186
  • Reputation:   51
  • Joined:  11/14/11
  • Last Seen:  

Totally agree with the clean up.

But we should talk about the implementation of the new struct.

As @Sirius_White pointed out,

PACKET(PKT_MC_CUTIN) pkt = { PKT_MC_CUTIN };

This is weird.

it is.

roBrowser currently send the oldest possible packet to server to get a maximum compatibility with all pserver (since ea/ra support almost all packets send from client to server).

Well I should improve robrowser to send the packet related to the detected packetver...

Well, nice idea but failed by design :D

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  11
  • Topics Per Day:  0.00
  • Content Count:  427
  • Reputation:   123
  • Joined:  11/17/11
  • Last Seen:  

I agree that, it is a bit wierd, and I think, meyraws codes are great reference, but maybe we could come up with something that is even cleaner based on that idea :D.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  169
  • Topics Per Day:  0.04
  • Content Count:  1260
  • Reputation:   750
  • Joined:  11/19/11
  • Last Seen:  

Totally agree with the clean up.

But we should talk about the implementation of the new struct.

As @Sirius_White pointed out,

PACKET(PKT_MC_CUTIN) pkt = { PKT_MC_CUTIN };

This is weird.

it is.

+1

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  25
  • Topics Per Day:  0.01
  • Content Count:  257
  • Reputation:   253
  • Joined:  11/29/11
  • Last Seen:  

PACKET(PKT_MC_CUTIN) pkt = { PKT_MC_CUTIN };

This is weird.

Redundant code is redundant.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  75
  • Topics Per Day:  0.02
  • Content Count:  2223
  • Reputation:   593
  • Joined:  10/26/11
  • Last Seen:  

Is any of the "clif cleanup" Ai4rei is committing in eAthena related to this idea?

I have not merged eAthena r15051-r15058 and I'm wondering if I should proceed or wait.

r15051

r15052

r15053

r15054

r15055 - already merged

r15056

r15057

r15058

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  11
  • Topics Per Day:  0.00
  • Content Count:  427
  • Reputation:   123
  • Joined:  11/17/11
  • Last Seen:  

Well, it is not related. Ai4rei did small cleanups, you can merge them, they are ok I think.

This one would be about a bigger rewrite how to handle packets.

Link to comment
Share on other sites


  • Group:  Members
  • Topic Count:  22
  • Topics Per Day:  0.00
  • Content Count:  392
  • Reputation:   285
  • Joined:  12/19/11
  • Last Seen:  

I'm totally in favor of improving or even rewriting client interface. But we really need to think it over thoroughly, how we're gonna do this.

Link to comment
Share on other sites

  • 10 months later...

  • Group:  Members
  • Topic Count:  169
  • Topics Per Day:  0.04
  • Content Count:  1260
  • Reputation:   750
  • Joined:  11/19/11
  • Last Seen:  

reviving this as proposed over #rdev;


One thing I'm highly uncomfortable with:

PACKET(PKT_MC_CUTIN) pkt = { PKT_MC_CUTIN };

since each packet version has its own code and length/sizes for its vars (e.g. check the inventory_list1/2/3 crap) I'd like to propose we use the packet id in the name e.g.

struct packet_0xf5 {
       char    offset[5];
       uint32  item_oid;
};
....
func() {
struct packet_0xf5 p;
p.item_oid = 1;
}

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...