Is there any problems in PveBotSystem.checkCorValidity?

[VictorECDSA]

I have some questions about this function and don't know if I understand it correctly.

contract PveBotSystem is System {
    function checkCorValidity(address player, uint32 x, uint32 y) internal view returns (bool hasErr) {
        // check x, y validity
        require(x < GameConfig.getLength(0), "x too large");
        require(y < GameConfig.getWidth(0), "y too large");

        // check whether (x,y) is empty
        uint256 cor = Coord.compose(x, y);
        // loop piece to check whether is occupied
        for (uint256 i = 0; i < Player.lengthHeroes(player); i++) {
            bytes32 key = Player.getItemHeroes(player, i);
            HeroData memory hero = Hero.get(key);
            if (cor != Coord.compose(hero.x, hero.y)) {
                hasErr = true;
            }
            // require(cor != Coord.compose(hero.x, hero.y), "this location is not empty");
        }

        hasErr = false;
    }

1. There is no 'return' branch to distinguish 'right' and 'wrong'. So the result of it will be always 'hasErr = false'.

2. The require sentence will lead to transaction failure.

3. The condition 'cor != Coord.compose(hero.x, hero.y' will be achieved at least one time, once if player have more than 2 heroes.

[ClaudeZsb]

hasErr = false seems like a cheat code for the reason of testing or getting PvE feature live quickly. @aLIEzsss4

And we did lack of careful review. Our bad.

This function has problems as @fenghaoming said. Besides, obviously the inner checking of position should be if (cor == Coord.compose(hero.x, hero.y)).