ChillerDragon / teeworlds_network

A teeworlds 0.7 network protocol library written in ruby
5 stars 0 forks source link

Design decisions #6

Closed ChillerDragon closed 1 year ago

ChillerDragon commented 1 year ago

Should the Context class call a proc in to_s to allow printing every context as a nice string by default? Or should all the Context data be replaced with a bunch of packet classes? Like lib/packets/*.rb and they all have the proper fields and a pack() and unpack() method.

ChillerDragon commented 1 year ago

Okay I gave it some thought and yes it is a good Idea to use classes representing packets. They should be annotated with "Server -> Client" or "Client -> Server" and they need to have the following methods.

to_a - To array/network representation

to_h - To hash

And two ways of initializing them. One using raw data and one using a hash. The hash should be verified and packed against the valid keys using a generic method that looks like this

    @start_info.each do |key, value|                                                                                                                                                                               
      if value.instance_of?(String)                                                                                                                                                                                
        data += Packer.pack_str(value)                                                                                                                                                                             
      elsif value.instance_of?(Integer)                                                                                                                                                                            
        data += Packer.pack_int(value)                                                                                                                                                                             
      else                                                                                                                                                                                                         
        puts "Error: invalid startinfo #{key}: #{value}"                                                                                                                                                           
        exit 1                                                                                                                                                                                                     
      end                                                                                                                                                                                                          
    end 

Not sure yet if it should be models/server_info.rb with class ServerInfo

or packets/server_info.rb with class PacketServerInfo

The former looks cool all over the place. The latter makes it obvious that this is a packet. When just setting the client name without sending a packet it would be weird to set a property of a packet class right?

ChillerDragon commented 1 year ago

StartInfo is a good start. Polish and test it. If it is good apply the format to all

https://github.com/ChillerDragon/teeworlds_network/blob/3358cc6608ff21e127b64f8a654b5b09b33731ca/lib/models/start_info.rb

ChillerDragon commented 1 year ago

Create rails like helper that generates the format like this

twnet g model client mode:int target_id:int message:str

# or
twnet g client_pck mode:int target_id:int message:str
twnet g server_pck mode:int target_id:int message:str

Which creates this template filled out with the fields https://github.com/ChillerDragon/teeworlds_network/blob/acba9e7a5d07360135d822574a37632daab6f742/lib/models/template.rb

Also check class name being upper camel case of lower snake case file name in bad_code.sh

ChillerDragon commented 1 year ago

Let's keep the context class but add a packet instance as attribute. Not inside of the data hash but directly on context.

Naming it PACKET tho is a bit weird since in reality it is just the data of one CHUNK. Imo having the term user facing is not very nice and packet is wrong.

ChillerDragon commented 1 year ago

Let's keep the context class but add a packet instance as attribute. Not inside of the data hash but directly on context.

Naming it PACKET tho is a bit weird since in reality it is just the data of one CHUNK. Imo having the term user facing is not very nice and packet is wrong.

lets go with "message" like tw does it which is the only non trash synonym for packet