Skip to main content ↑ ↓
Topic: signed/unsigned int cleanup. (Read 5572 times) previous topic - next topic

signed/unsigned int cleanup.

Right now vehicle/convoy speed-related fields use sint32 as well as uint32.

Code: [Select]
/boden/wege/weg.h:     uint16 max_speed;
./bauer/vehikelbauer.h: static sint32 get_speedbonus( sint32 monthyear, waytype_t wt );
./simworld.h:   int average_speed[8];
./simworld.h:   int get_average_speed(waytype_t typ) const { return average_speed[ (typ==16 ? 3 : (int)(typ-1)&
./simworld.h:   sint32 get_record_speed( waytype_t w ) const;
./simworld.h:   void notify_record( convoihandle_t cnv, sint32 max_speed, koord pos );
./vehicle/simvehikel.h: uint32 speed_limit;
./simconvoi.h:  uint32 speed_limit;
./simconvoi.h:  sint32 min_top_speed;
./simconvoi.h:  sint32 max_record_speed; // current convois fastest speed ever
./simconvoi.h:  sint32 akt_speed_soll;    // target speed
./simconvoi.h:  sint32 akt_speed;               // current speed
./simconvoi.h:  const sint32& get_akt_speed() const { return akt_speed; }
./simconvoi.h:  const sint32 & get_min_top_speed() const {return min_top_speed;}
./gui/convoi_info_t.h:  sint32 mean_convoi_speed;
./gui/convoi_info_t.h:  sint32 max_convoi_speed;
It lead to annoying warnings and errors. I want replace uint32 with sint32. Any objections?

Re: signed/unsigned int cleanup.

Reply #1 –
Not from my side. So you are going to change everything speed-related to sint32?

Edit: besides the variables you listed, there are a couple of them in the besch/* files, eg uint16 vehikel_besch_t::geschw, some uint32 topspeed etc. /edit

There is also some related discussion here:
http://forum.simutrans.com/index.php?topic=4826.0
Parsley, sage, rosemary, and maggikraut.

Re: signed/unsigned int cleanup.

Reply #2 –
Yes. Unsigned integer wrap point (0) is really inconvenient.

Re: signed/unsigned int cleanup.

Reply #3 –
changing all int, uint, uin26 ... to sint32 would also make most sense to me. However, since there are some shift oprations involved, some casts to unsigned may be needed at certain places.

Or one could handle the speed as a float, since this routine is not the most limiting factor anymore, imho. But that is rather log term.

Re: signed/unsigned int cleanup.

Reply #4 –
Quote from: prissi – on
changing all int, uint, uin26 ... to sint32 would also make most sense to me.
How sensible code to objects size? Can uint16 weg_t::max_speed be replaced?

Quote from: prissi – on
However, since there are some shift oprations involved, some casts to unsigned may be needed at certain places.
The only shift i find so far is
Code: [Select]
simconst.h:#define speed_to_kmh(speed) (((speed)*VEHICLE_SPEED_FACTOR+511) >> 10)
simconst.h:#define kmh_to_speed(speed) (((speed) << 10) / VEHICLE_SPEED_FACTOR)
it can be replaced with '*' and '/'

Re: signed/unsigned int cleanup.

Reply #5 –
I think all stuff dealing with speed should behave the same to avoid conversions etc. Weg_t max_speed is usually queried by get_max... which then should also return an sint32. For storage, I would keep the current format.

The shifts look ok, so no need seem needed. I just though in simconvoi.cc there ws some bit shifting done, which will ge wrong results hen negative.

Re: signed/unsigned int cleanup.

Reply #6 –
I will not touch it, but does write_sint32 works as expected for different endianness?
What endianess used in pak files?

From besch/writer/obj_node.h
Code: [Select]
cl**** obj_node_t {
 [SKIP]
                void write_sint8(FILE* fp, sint8 data, int offset) {
                        this->write_uint8(fp, (uint8) data, offset);
                }
                void write_sint16(FILE* fp, sint16 data, int offset) {
                        this->write_uint16(fp, (uint16) data, offset);
                }
                void write_sint32(FILE* fp, sint32 data, int offset) {
                        this->write_uint32(fp, (sint32) data, offset);
                }
From besch/writer/obj_node.cc:
Code: [Select]
void obj_node_t::write_data_at(FILE* fp, const void* data, int offset, int size)
{
        if (offset < 0 || size < 0 || offset + size > desc.size) {
                char reason[1024];
                sprintf(reason, "invalid parameters (offset=%d, size=%d, obj_size=%d)", offset, size, desc.size);
                throw obj_pak_exception_t("obj_node_t", reason);
        }
        fseek(fp, write_offset + offset, SEEK_SET); 
        fwrite(data, size, 1, fp);
}
void obj_node_t::write_uint32(FILE* fp, uint32 data, int offset)
{
        uint32 data2 = endian(data);
        this->write_data_at(fp, &data2, offset, 4);
}

Re: signed/unsigned int cleanup.

Reply #7 –
Imho, simutrans writes/reads all data in Intel byte order (little endian).
Parsley, sage, rosemary, and maggikraut.

Re: signed/unsigned int cleanup.

Reply #8 –
... but the write routines should take care of that.

Re: signed/unsigned int cleanup.

Reply #9 –
patch posted