Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Will the synergy effect if counter exceed 4 but only with 2-count configed? #200

Open
VictorECDSA opened this issue Dec 11, 2023 · 6 comments

Comments

@VictorECDSA
Copy link
Contributor

For example, about the RaceSynergyEffect of troll, only config 2-count (no 4-count):

library EffectInitializer {
    function _initSynergy(IWorld _world) private {
        //     troll: + 10% attack the enemy twice
        RaceSynergyEffect.set(0x000002, 0, 0x1c80_ff);
contract PieceInitializerSystem is System {
    function _addRaceSynergy(uint256 _counter, uint256 _effects, uint256 _enemyEffects)
        private
        view
        returns (uint256, uint256)
    {
        for (uint256 i; i < RACE_NUMBER; ++i) {
            if (count / (4 * base) > 0) {
                data = RaceSynergyEffect.get(4 * base);
            } else if (count / (2 * base) > 0) {
                data = RaceSynergyEffect.get(2 * base);
            }
            if (data.effect > 0) {
                ......
            }

Lets say the count if bigger than 4, then the judgement 'count / (4 * base) > 0' is true, but 'data = RaceSynergyEffect.get(4 * base)' is empty. Even though 2-count effect has been configed, but the result is no effect.

Don't know if this is in line with the intent of the design, thank you!

@aLIEzsss4
Copy link
Collaborator

Join us on Discord for the latest news.

Qget5JQHtr

@noyyyy
Copy link
Member

noyyyy commented Dec 12, 2023

@ClaudeZsb

@VictorECDSA
Copy link
Contributor Author

Join us on Discord for the latest news.

Qget5JQHtr

Thanks, I have join the discord.

@VictorECDSA
Copy link
Contributor Author

VictorECDSA commented Dec 12, 2023

@ClaudeZsb

Thanks for responding. I don't know whether I have misunderstood. If not, how about change code like this:

    function _addRaceSynergy(uint256 _counter, uint256 _effects, uint256 _enemyEffects)
        private
        view
        returns (uint256, uint256)
    {
        for (uint256 i; i < RACE_NUMBER; ++i) {
            RaceSynergyEffectData memory data;
            if (count / (4 * base) > 0) {
                data = RaceSynergyEffect.get(4 * base);
            }
            if (data.effect <= 0 && count / (2 * base) > 0) {
                data = RaceSynergyEffect.get(2 * base);
            }
            if (data.effect > 0) {
                ... ...
            }

@ClaudeZsb
Copy link
Collaborator

@ClaudeZsb

Thanks for responding. I don't know whether I have misunderstood. If not, how about change code like this:

    function _addRaceSynergy(uint256 _counter, uint256 _effects, uint256 _enemyEffects)
        private
        view
        returns (uint256, uint256)
    {
        for (uint256 i; i < RACE_NUMBER; ++i) {
            RaceSynergyEffectData memory data;
            if (count / (4 * base) > 0) {
                data = RaceSynergyEffect.get(4 * base);
            }
            if (data.effect <= 0 && count / (2 * base) > 0) {
                data = RaceSynergyEffect.get(2 * base);
            }
            if (data.effect > 0) {
                ... ...
            }

You're right. This does fix the bug. Great thanks. Would you like to create a PR for fixing it?

And for the second if, I would prefer data.effect == 0 &&....

@VictorECDSA
Copy link
Contributor Author

@ClaudeZsb

Thanks for responding. I don't know whether I have misunderstood. If not, how about change code like this:

    function _addRaceSynergy(uint256 _counter, uint256 _effects, uint256 _enemyEffects)
        private
        view
        returns (uint256, uint256)
    {
        for (uint256 i; i < RACE_NUMBER; ++i) {
            RaceSynergyEffectData memory data;
            if (count / (4 * base) > 0) {
                data = RaceSynergyEffect.get(4 * base);
            }
            if (data.effect <= 0 && count / (2 * base) > 0) {
                data = RaceSynergyEffect.get(2 * base);
            }
            if (data.effect > 0) {
                ... ...
            }

You're right. This does fix the bug. Great thanks. Would you like to create a PR for fixing it?

And for the second if, I would prefer data.effect == 0 &&....

Yes, I would like create a PR for it. It is my honor to join such a charming project !

And for the second 'if', it is my coding habit that use '<=' or '>=' instead of '=='. I think it can avoid boundary risks in special cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants