bugfix: Prevent ranking of riders that are not trainable #1970
Conversation
| return FALSE; | ||
|
|
||
| #if !RETAIL_COMPATIBLE_CRC | ||
| const ContainModuleInterface* contain = other->getContain(); |
| return FALSE; | ||
|
|
||
| // Sorry, you can't gain levels | ||
| if( !other->getExperienceTracker()->isTrainable() ) |
There was a problem hiding this comment.
Seeing these conditions now used at 2 places, how about add a function in Object for "eligibleForLevelUp" and then call that here and in CommandXlat? This way the conditions stay in sync.
There was a problem hiding this comment.
I did consider this, but as one of the usages is debug logic, I thought it would not really be that useful.
There was a problem hiding this comment.
I probably would add a Object::isTrainable, since it has this intricacy with the Rider.
Stubbjax
force-pushed
the
fix-rider-trainability
branch
from
30f557c to
fce6f1d
Compare
xezon
left a comment
xezon
left a comment
There was a problem hiding this comment.
I wonder if this should be fixed a different way. There is explicit code for transfering experience from rider to bike and from bike to rider:
//Transfer experience from the rider to the bike.
ExperienceTracker *riderTracker = rider->getExperienceTracker();
ExperienceTracker *bikeTracker = obj->getExperienceTracker();
bikeTracker->setVeterancyLevel( riderTracker->getVeterancyLevel(), FALSE );
riderTracker->setExperienceAndLevel( 0, FALSE );//Transfer experience from the bike to the rider.
ExperienceTracker *riderTracker = rider->getExperienceTracker();
ExperienceTracker *bikeTracker = bike->getExperienceTracker();
riderTracker->setVeterancyLevel( bikeTracker->getVeterancyLevel(), FALSE );
bikeTracker->setExperienceAndLevel( 0, FALSE );Maybe the trainable flag needs to be transfered as well, so that the bike is not trainable while mounted by a worker?
This way the modified trainable tests would be no longer necessary. The bike would simply inherit the workers experience.
| ExperienceTracker* getExperienceTracker() {return m_experienceTracker;} | ||
| const ExperienceTracker* getExperienceTracker() const {return m_experienceTracker;} | ||
| VeterancyLevel getVeterancyLevel() const; | ||
| Bool isTrainable() const; //< Can this unit - or its rider - gain veterancy? |
|
|
||
| const ExperienceTracker *et = other->getExperienceTracker(); | ||
| if( !et || !et->isTrainable() ) | ||
| if (!et || !other->isTrainable()) |
There was a problem hiding this comment.
Remove !et test and move const ExperienceTracker *et = other->getExperienceTracker(); down.
|
|
||
| const ExperienceTracker *et = other->getExperienceTracker(); | ||
| if( !et || !et->isTrainable() ) | ||
| if (!et || !other->isTrainable()) |
There was a problem hiding this comment.
Looking at the code in this function, I am confused now.
What about:
const ExperienceTracker *et = other->getExperienceTracker();
if (!et || !et->canGainExpForLevel(levelsToGain))Shouldn't that also account for the Rider to work properly?
Maybe getExperienceTracker needs to be changed to return the experience tracker of the rider if it has one?
2 participants
This change fixes an issue that allows riders to rank up even if they are not trainable. For example, a player might place a Worker on a Combat Cycle and then drive over a Salvage Crate.
Before
A mounted Worker ranks up when driving over a Salvage Crate
RANKED_WORKER.mp4
After
Only trainable mounted units can rank up
TRAINABLE_VET.mp4
Note: The changes to Generals are solely for unification. Riders do not exist there.