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

Added support for custom Hero and Hero Power textures #373

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

guyde2011
Copy link

This is one of the few gui tweaks I had on my local version - it takes textures from the <metastone_folder>/assets and loads them.
Keep in mind that it does not break any cards - when there isn't any texture given it's going to use the basic hero power texture of the played class.

Here's an example for an updated Jaraxxus card code:

"name": "Lord Jaraxxus",
"baseManaCost": 9,
"heroPower": "hero_power_inferno",
"type": "HERO",
"asset": "jaraxxus",
"heroClass": "WARLOCK",
"rarity": "LEGENDARY",
"race": "DEMON",
"attributes": {
	"HP": 15,
	"MAX_HP": 15
},
"collectible": false,
"set": "CLASSIC",
"fileFormatVersion": 1

Pretty much the same as it used to be with the additional "asset": "jaraxxus" line which points to <metastone_folder>/assets/jaraxxus.png

Allowed HeroCard to support custom assets
Allowed for custom assets support when parsing a hero card
Allowed HeroPower to be parsed with a custom asset
Allowed for custom Hero Power assets
}
return iconPath + ".png";
} else {
String assetsFolder = UserHomeMetastone.getPath().replace("\\\\", "/") + "/" + "assets";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably use File.separator for compatibility with filesystems that use a file separator other than "/". Ran into this problem when I refactored the CardCatalogue to load cards from the user's home metastone dir.

iconPath += "unknown";
break;

if (heroPower.getAsset().equals("<Def>")){
Copy link
Contributor

@akoscz akoscz Aug 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming you declared <Def> to mean default value? Could we not use null to mean default value? In which case this becomes a null check rather than a string comparison. A tad bit more efficient :) If I misunderstood the intention of <Def> or I have overlooked something in your PR as to why it's needed, please explain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@webadict @demilich1 what do you guys think about using a string such as <Def> to represent default value in an condition statement? To me this feel wonky.

null in this case makes sense as a default value. It means there was no asset available for the heroPower. This is the only place this condition is checked, so there's no need to complicate things for future proofing if theres no other scenario where asset is used in a condition.

@akoscz
Copy link
Contributor

akoscz commented Aug 3, 2017

does the addition of the asset attribute to the card format constitute an increment of the fileFormatVersion ?

@guyde2011
Copy link
Author

@akoscz First of all, using null as a default value is bad practice - in the future it could lead to some problems - using a default value other than null (<Def>) means that if we have a loading problem which causes the loaded string to be null than we would not be able to track it if we were to use null.

For the fileFormatVersion - I felt that it wasn't really a big change as it is able to load the already existing files.

For the file separator - yeah I should patch it - did you patch it on your local copy? (if you did can you upload it?)

@webadict
Copy link
Collaborator

webadict commented Aug 3, 2017

fileFormatVersion has like... zero use right now. It would be nice to use eventually.

@akoscz
Copy link
Contributor

akoscz commented Aug 5, 2017

@guyde2011 re: File.separator, take a look at the usage in CardCatalogue.java.
I have not patched it on my local copy. I'll kindly ask you to make the change and update your PR. :) Thanks!

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

Successfully merging this pull request may close these issues.

3 participants