Skip to content

Craftconomy3 support#22

Open
elgarfo wants to merge 1 commit into
fernferret:masterfrom
elgarfo:master
Open

Craftconomy3 support#22
elgarfo wants to merge 1 commit into
fernferret:masterfrom
elgarfo:master

Conversation

@elgarfo

@elgarfo elgarfo commented Jan 7, 2013

Copy link
Copy Markdown

No description provided.

@fernferret

Copy link
Copy Markdown
Owner

Sorry for the lack of replies, wanted to pop in and say I'll be looking at it this weekend, thanks for the PR!

@fernferret

Copy link
Copy Markdown
Owner

Oh man... @krinsdeath Have you had a chance to look at this? I feel terrible for not getting to it...

@krinsdeath

Copy link
Copy Markdown
Collaborator

I'll have to write a module for it since I had to refactor the project to include support for both new and old versions of plugins with changed APIs (like Fe-economy).

I apologize @elgarfo ! I never noticed you had written this.

@fernferret

Copy link
Copy Markdown
Owner

It's funny, because @krinsdeath is apologizing for something I said I'd do... 👎 goes to @fernferret.

@elgarfo

elgarfo commented Mar 13, 2013

Copy link
Copy Markdown
Author

just had again a quick look at the code, because i remembered CC3 has changed some bits. i also noticed, that i am using getCurrencyNames.get(0) which might not be the best option.
it's technically possible to have only one currency registered in CC3 that is not having an id of 0.
but i haven't looked at the returnvalue of getCurrencyNames(), perhaps it is a zero-based list which is returned.
anyway. better approach could be to use getCurrencyManager().getDefaultCurrency() (IIRC), that would be the more logical approach i think.
should i file a new PR with updated code, or do you want to fix it yourselves while importing this, possibly outdated, PR?

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