b-casey / OpenSMACX

A project to decompile SMAC/X to C++ with the long term goal of creating a full open source clone.
GNU General Public License v3.0
52 stars 4 forks source link

Minor bug: Gender switching when clicking in base window #31

Open pianoslum opened 3 years ago

pianoslum commented 3 years ago

A very very minor bug I've always found rather amusing: If you click on the base tile in a base's resource screen or change to psych view, all drones, workers and talents get a random gender assigned.

b-casey commented 3 years ago

I believe the gender assignment is completely random. If you open and close a base a bunch of times, the current talent/drone/workers will shift to different genders. That said, once you have a set it should probably stay uniform across the base UI for that instance. Likely something inside BaseWin class that handles base UI window.

pianoslum commented 3 years ago

Yeah I agree. Nobody will notice or care if the genders are different after closing and opening the base UI, but when you're in it it's noticeable. I guess there is some kind of random assignment in the background, so maybe on could initialize the RNG with some value unique to the base (e.g. base id or memory address) to have it consistent.

DrazharLn commented 3 years ago

Is there just one global RNG? If so, seeding it when you draw the citizens could lead to exploits unless the old RNG state is restored after drawing.

On Tue, 26 Jan 2021, 20:02 pianoslum, notifications@github.com wrote:

Yeah I agree. Nobody will notice or care if the genders are different after closing and opening the base UI, but when you're in it it's noticeable. I guess there is some kind of random assignment in the background, so maybe on could initialize the RNG with some value unique to the base (e.g. base id or memory address) to have it consistent.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/b-casey/OpenSMACX/issues/31#issuecomment-767793491, or unsubscribe https://github.com/notifications/unsubscribe-auth/AESXNWPGA6GOLY2FB7VUFVTS34NWXANCNFSM4WSS5YWQ .

pianoslum commented 3 years ago

I don't know... We shouldn't tamper with the global RNG of course. But I'm sure there is a simple solution if we just use some id for a base to create some pseudo-random bitstring.

DrazharLn commented 3 years ago

I think it's fine to tamper with the global RNG so long as you restore its state. There won't be another thread pulling values from it at the same time, probably.

But, yeah, probably easier to just hash a base id and use those bits or write a tiny bad PRNG and seed it with a base id. Simple PRNG: https://en.wikipedia.org/wiki/ACORN_(PRNG). Some suitable integer hashing functions are here: https://gist.github.com/badboy/6267743 Or we could use djb2 to hash byte-by-byte.

On Wed, 27 Jan 2021 at 13:27, pianoslum notifications@github.com wrote:

I don't know... We shouldn't tamper with the global RNG of course. But I'm sure there is a simple solution if we just use some id for a base to create some pseudo-random bitstring.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/b-casey/OpenSMACX/issues/31#issuecomment-768284274, or unsubscribe https://github.com/notifications/unsubscribe-auth/AESXNWOJUYOGEE3TIOUXXFTS4AIDDANCNFSM4WSS5YWQ .

b-casey commented 3 years ago

It looks like it uses the Random class for both generations so scope of PRNG should be pretty localized. It might be possible to change it so both use the same reseed value. One would have to confirm that no other components use Random class for
draw_pop and psych_row.

Initial drawing in base UI: 00414B40 ; void __thiscall BaseWin::draw_pop(BaseWin *this, int) 00414B8B call ?reseed@Random@@QAEXK@Z ; Random::reseed(ulong)

Drawing of psych panel: 00408B20 ; void __thiscall BaseWin::draw_psych(BaseWin this) 00408720 ; void __thiscall BaseWin::psych_row(BaseWin this, int, int, int, int, int, int, int) 00408790 call ?reseed@Random@@QAEXK@Z ; Random::reseed(ulong)

induktio commented 3 years ago

It has to be noted that base and vehicle Ids are NOT permanent. When base ID=x gets destroyed, all the IDs > x will get decremented by one. One way to work around this is to create a simple hash function that gets seeded with the base coordinates and maybe map_random_seed value to shuffle it further. Then it is guaranteed to always produce the same results for a particular tile.

pianoslum commented 2 years ago

This was solved in thinker, btw. Maybe you could cherry-pick the edits.