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

Ok closing this does not delete it. I thought that's what it did. Since this had to be revised delete this one or whatever it is that is done with ones that aren't going to be added to xmage. #2534

Closed
wants to merge 2 commits into from

Conversation

@ghost
Copy link

@ghost ghost commented Oct 31, 2016

This is my first time contributing to xmage. Go easy on me if I made any mistakes. I hope to be able to contribute a lot of cards moving forward.

@Override
public boolean replaceEvent(GameEvent event, Ability source, Game game) {
PreventionEffectData preventionEffectData = preventDamageAction(event, source, game);
if (preventionEffectData.getPreventedDamage() > 0) {

This comment has been minimized.

@emerald000

emerald000 Oct 31, 2016
Contributor

The counter should be removed even if no damage was actually prevented (with Wild Slash for example).

This comment has been minimized.

@ghost

ghost Oct 31, 2016
Author

Protean Hydra has the exact same ability though phrased differently as the Rock Hydra. So this was a simple copy and paste. I assume that whatever change that needs to be made for Rock Hydra would also need to be made for Protean Hydra. So if I am understanding correctly then the change is as follows:

if (preventionEffectData.getPreventedDamage() >= 0) {

I didn't think about it but how about damage that is prevented in full or in part with Rock Hydra's {R}: Prevent the next 1 damage that would be dealt to Rock Hydra this turn. I was unable to test the card because the AI refused to attack or block me if I had a Rock Hydra in play.

This comment has been minimized.

@emerald000

emerald000 Oct 31, 2016
Contributor

The prevented damage will always be >=0, so you should drop the if statement completely. You can also get the damage that is tried to be applied with event.getAmount(). That will let you remove a number of counters equal to the damage assigned, not only prevented.

If a previous replacement effect replaced the effect entirely, no other replacement effects are fired. If it replaces it in part, it will change the event.getAmount() to the new amount.

The AI can be finicky sometimes. You might have better luck with running two instances of the client and making a player vs. player game.

PS: You might be interested in joining our Gitter room (https://gitter.im/magefree/mage). Most developers hang in there and can help you if you have questions.

This comment has been minimized.

@ghost

ghost Oct 31, 2016
Author

I think I may have gotten confused so I am hoping that this is the correct way to do it. The code for that should be as follows then?:

public boolean replaceEvent(GameEvent event, Ability source, Game game) {
    Permanent permanent = game.getPermanent(source.getSourceId());
    if (permanent != null) {
            permanent.removeCounters(CounterType.P1P1.createInstance(event.getAmount()), game);
        }
        return false;
}

This comment has been minimized.

@emerald000

emerald000 Oct 31, 2016
Contributor

You'll still want the preventDamageAction to do the actual damage preventing. Looks fine otherwise. Make sure to test though as fine-looking code isn't always working. ;)

This comment has been minimized.

@ghost

ghost Oct 31, 2016
Author

I'm learning this on the fly so hopefully I have gathered how it works correctly with the correction being?:

public boolean replaceEvent(GameEvent event, Ability source, Game game) {
            preventDamageAction(event, source, game);
            Permanent permanent = game.getPermanent(source.getSourceId());
            if (permanent != null) {
                permanent.removeCounters(CounterType.P1P1.createInstance(event.getAmount()), game);
            }
            return false;
}

This comment has been minimized.

@emerald000

emerald000 Oct 31, 2016
Contributor

You got it. Try to prevent the damage, then remove the counters no matter if it was prevented.

This comment has been minimized.

@ghost

ghost Nov 1, 2016
Author

The test did not go as expected. I cast the card fine with one +1/+1 counter on it. I played against myself and attacked with a 1/1 creature and blocked with the hydra. I did not activate any of the mana abilities and the hydra should have lost the counter and died but it remained in play with one counter.

This comment has been minimized.

@ghost

ghost Nov 2, 2016
Author

Ok I have it working now. So I should close this request so that I just have the new pull request with the corrected changes?

@Override
public boolean replaceEvent(GameEvent event, Ability source, Game game) {
PreventionEffectData preventionEffectData = preventDamageAction(event, source, game);
if (preventionEffectData.getPreventedDamage() > 0) {

This comment has been minimized.

@LevelX2

LevelX2 Nov 1, 2016
Contributor

Check your settings for line endings.
The set files shouldn't be replaced with all lines.
There should only be the added line changed.
On windows mine is set to: core.autocrlf=false
So enter e.g. to change the setting:
git config core.autocrlf false

This comment has been minimized.

@ghost

ghost Nov 2, 2016
Author

I entered that at the command prompt. Will this also apply to doing commits and pushing from within netbeans? And is there something I have to do to push those set files again with this change? When I select commit in netbeans only the Rock Hydra file gets picked by netbeans unlike previously the set files were also in the list of files to commit.

This comment has been minimized.

@LevelX2

LevelX2 Nov 2, 2016
Contributor

I normally don't create pull requests, so I'm not familiar with that.
Maybe you have to manually touch the files again, to apply the line endings change.

This comment has been minimized.

@spjspj

spjspj Nov 2, 2016
Contributor

@MTGfan If my fork gets in a muddle on github, I make sure I've saved the bit of work I want to keep externally, then delete my fork, and create a new one. It should be up-to-date with the main magefree branch then. Then you can use something external to netbean's internal git (I found and like 'Git extensions - Visual Studio and Shell Explorer Extensions for Git') and then just upload the two files (the set file and the implementation of the card file) and then create a new pull request from that.

@ghost ghost closed this Nov 2, 2016
@ghost ghost changed the title This is my first card contribution. Ok closing this does not delete it. I thought that's what it did. Since this had to be revised delete this one or whatever it is that is done with ones that aren't going to be added to xmage. Nov 2, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants