MineColonies

MineColonies

53M Downloads

Question: Effect of housing on happiness according to the code

Amadeus99 opened this issue ยท 7 comments

commented

Minecolonies version

Curse 0.7.984

Expected behavior

Housing has an effect on overall happiness regardless of number of citizens.

Actual behaviour

First understand that my understanding of the code is a bit basic. I was looking to try and figure out what affected happiness since my happiness has been stuck at 1 for a long time.

It looks like to me that in https://github.com/Minecolonies/minecolonies/blob/version/1.10/src/main/java/com/minecolonies/coremod/colony/Colony.java, in updateOverallHappiness() starting at line 1189, that the "housing" variable will only ever be set to the building level of the home of the last citizen checked in the loop at line 1194:
for(final CitizenData citizen: citizens.values())
by line 1212:
housing = home.getBuildingLevel();

After the end of the loop, average housing is set by line 1218:
final int averageHousing = housing/ Math.max(1, citizens.size());

All of this means to me that "housing" will never be higher than 5 (the max building level) and as such if you have 5 or more citizens, "averageHousing" will always be <= 1 and will never affect overall happiness because "averageHousing" has to be > 1 to increase overall happiness (starting at line 1220):
if(averageHousing > 1)
{
increaseOverallHappiness(averageHousing * HAPPINESS_FACTOR);
}

If I'm reading this incorrectly and the code is as it should be, please say so and close this ticket.

Thanks.

commented

Yeah, I would expect

housing = home.getBuildingLevel();

should have been

housing += home.getBuildingLevel();
commented

I don't think that would work since a house can have multiple citizens living in it which means that houses would get counted multiple times.

Edit: If I'm right, then you would have to keep track of which houses had already been counted, or use a separate loop on buildings to only count the houses once.

commented

but both citizens would both be happy they have a level 5 house would they not?
So since they are both happy, it should count twice. It get's divided by people at the end. So 2 people having house 5 = 10; Then that happy 10 / 2 citizens = 5;

Otherwise as you have pointed out, it's (last citizen) = house happy 5. Then that happy 5 / 2 citizens = 2.5;

commented

@WORMSS I think you are correct, could you do a pr ?

commented

@xavierh I thought I already had.. Seems I went through the process of making the changes, and writing up the pull request.. but never pressed the submit button.. (D'oh).. done it again now.

EDIT: yep Double D'oh, I just found the tab on another Chrome Window... Oh well.. Done now.

commented

Actually I don't think this is a correct fix.
We should look on Citizen Hut, doing the sum of level then average it but the number of citizen.

commented

@WORMSS See my comment on pr and you will need to sign the CLA