search

afritz1 / OpenTESArena

Open-source re-implementation of The Elder Scrolls: Arena.
MIT License
915 stars 68 forks source link

Citizen color variations aren't close enough to original game #187

Closed afritz1 closed 3 years ago

afritz1 commented 3 years ago

While several citizens' colors look just fine, like the female citizens in mountain regions, others seem to not have the same variety as the original game. Citizens in temperate climates for example all seem to have the same default hair color. Maybe there is something wrong with the citizen color transformation function.

https://github.com/afritz1/OpenTESArena/blob/2992be2ca7c5d634c9249a739125ff0b0777cf98/OpenTESArena/src/Assets/ArenaAnimUtils.cpp#L1287

Carmina16 commented 3 years ago

Have you checked the input data? seed must be unique adnd individual for each NPC, colorBase should be 32, 42, 52, 64, 74, 84, 100, 116, 128, 138, 148, 160, 170, 180, 202, 212.

afritz1 commented 3 years ago

The seed is being obtained from the modern RNG for each citizen and colorBase appears to be correct. https://github.com/afritz1/OpenTESArena/blob/30ec980126153b1687cc2c970bbe6fb3b85db2c3/OpenTESArena/src/Entities/CitizenManager.cpp#L173

Allofich commented 3 years ago

It seems like this line

newPalette[newIndex] = palette[oldIndex];

should be newPalette[oldIndex] = palette[newIndex];

afritz1 commented 3 years ago

That gives a bit better results. It's hard to tell because of randomization but I think that's the correct change. I saw some citizens with blue hair but I think that's fine? screenshot000

Allofich commented 3 years ago

I don't know for 100% but that screenshot looks OK to me.

Also I just checked the original game and found a citizen with blue hair. Screenshot 2020-09-16 020702

afritz1 commented 3 years ago

Okay, I will look at this again later tonight and merge in probably.

afritz1 commented 3 years ago

Fixed in commit 9ca3699adb393d802d16173bbf4fd3f22f33e3bc. Thanks for the help.