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

Configuration: Currency, Starting Coins, Coin Value, and French translation #92

Merged
merged 12 commits into from
Jul 26, 2024

Conversation

Daudeuf
Copy link
Contributor

@Daudeuf Daudeuf commented Jul 8, 2024

New config file "numismatics-common.toml":

  • Custom current currency
  • Starter coins for new player accounts
  • Custom coins values
  • And missing french translations

@Daudeuf
Copy link
Contributor Author

Daudeuf commented Jul 8, 2024

@IThundxr can you post it on Curseforge when you've checked everything?

@Daudeuf Daudeuf changed the title Configuration: Currency, Starting Coins, and Coin Value Configuration: Currency, Starting Coins, Coin Value, and French translation Jul 8, 2024
@banna12345bob
Copy link
Contributor

Addresses #91 & #39.

IThundxr added 2 commits July 21, 2024 11:33
…udeuf/1.20.1/dev

# Conflicts:
#	common/src/main/java/dev/ithundxr/createnumismatics/content/backend/Coin.java
#	common/src/main/java/dev/ithundxr/createnumismatics/content/vendor/VendorBlockEntity.java
@IThundxr IThundxr requested review from techno-sam and IThundxr July 21, 2024 15:38
@IThundxr
Copy link
Member

Note: after some discussion we feel changing the coin values currently isn't a great idea as it could get confusing for users that play on multiple servers

@Daudeuf
Copy link
Contributor Author

Daudeuf commented Jul 21, 2024

It's true that changing the values between servers can be confusing. However, the current values are already confusing. I tried to introduce this into the configuration system I proposed because the players on my server were already lost.

The current values are too different from the system present in real life with divisions by multiples of 8. It would be better to have divisions by multiples of 10 and to ensure that cents are represented by only 2 objects instead of 3. This would allow for higher savings on the servers and be more representative for the players.

Finally, I understand that changing the default values at this stage of mod development is complicated, but introducing this in the configuration would be a real asset.

@IThundxr IThundxr requested a review from techno-sam July 22, 2024 14:16
Copy link
Member

@techno-sam techno-sam left a comment

Choose a reason for hiding this comment

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

One nitpick and then this is good to merge

public final ConfigInt starterSprockets = i(0, 0, "starterSprockets");
public final ConfigInt starterCogs = i(0, 0, "starterCogs");
public final ConfigInt starterCrowns = i(0, 0, "starterCrowns");
public final ConfigInt starterSuns = i(0, 0, "starterSuns");
Copy link
Member

Choose a reason for hiding this comment

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

the string names should still be underscored, I think. (e.g. p f CI starterSuns = i(0, 0, "starter_suns");)

@IThundxr IThundxr merged commit 7931e3e into Layers-of-Railways:1.20.1/dev Jul 26, 2024
1 check passed
@BredyAK
Copy link

BredyAK commented Oct 6, 2024

It's true that changing the values between servers can be confusing. However, the current values are already confusing. I tried to introduce this into the configuration system I proposed because the players on my server were already lost.

The current values are too different from the system present in real life with divisions by multiples of 8. It would be better to have divisions by multiples of 10 and to ensure that cents are represented by only 2 objects instead of 3. This would allow for higher savings on the servers and be more representative for the players.

Finally, I understand that changing the default values at this stage of mod development is complicated, but introducing this in the configuration would be a real asset.

good point.

i feel sorry for this mod because the developer's stubbornness has severely limited its functionality. many users have submit similiar feature requests but all of them are rejected, the devs just keep their own point.

#39

@techno-sam
Copy link
Member

Bredy, excuse me! I do so deeply apologize that having a unified creative vision for the mod and spending more than a year implementing features is considered "stubbornness" and "limited [...] functionality". I greatly appreciate this PR, it is genuinely good, it just had one change that didn't fit our vision. That's OK. I don't see anyone submitting ad hominem attacks to the Create devs because the gear ratio between large and small gears isn't configurable. Comments like yours are what, over time, make developers quit Minecraft modding.

Also, the mod is under a fairly permissive license. If you truly need customizable coin values for your use, feel free to make those changes yourself in your copy, rather than submitting vaguely hostile and rude comments without any contribution of work.

Before your comment, I would have been willing to discuss this with the PR author. Now, the very idea of doing so is offensive to me.

@BredyAK
Copy link

BredyAK commented Oct 7, 2024

Bredy, excuse me! I do so deeply apologize that having a unified creative vision for the mod and spending more than a year implementing features is considered "stubbornness" and "limited [...] functionality". I greatly appreciate this PR, it is genuinely good, it just had one change that didn't fit our vision. That's OK. I don't see anyone submitting ad hominem attacks to the Create devs because the gear ratio between large and small gears isn't configurable. Comments like yours are what, over time, make developers quit Minecraft modding.

Also, the mod is under a fairly permissive license. If you truly need customizable coin values for your use, feel free to make those changes yourself in your copy, rather than submitting vaguely hostile and rude comments without any contribution of work.

Before your comment, I would have been willing to discuss this with the PR author. Now, the very idea of doing so is offensive to me.

I'm sorry to offend you, and I apologize for the stubborn word.

In the discord, my questions were selectively ignored, which made me feel the attitude of the dev team was contemptuous. And, another dev like to post "¯_(ツ)_/¯" under my posts, this makes me feel offended, and it is the direct source of my negative emotions. Again, I apologize for that.

For this feature request, I did a deeply search both on discord and github, and find so many people submitted the feature request about custom value for different currencies, but most of then are closed without any reason. (just like #39) and, in discord, the dev just told us he did a talk about this feature within the dev team and they say it does not fit the team's design style and may cause confusion for player's on different servers. TBH, why not let player's vote for this? This is just like Mojang believes the mob voting event fits their vision. But it brings more hatred to the community than it's worth. This is the only thing I want to say, the dev team should also listen to community's voice. This is not meaning you must accept all parts of this PR, I just mean you should ask for the community if it is mentioned by many people. But in any case, the decision is yours. I am just offering my suggestions.

Anyway, this is a great PR, it's such a pity to see the custom value part is rejected, and you are also the person worthy of my respect, because you bring the great work to the community, I thank you for that.

Sorry but I'm not going to use this mod because the lack of the custom value feature, and best wishes it could be added in future, i'll come back then. And wish this mod could have a brighter future.

@Violet-Scarelli
Copy link
Contributor

It certainly does have a bright future. Thank you for the words, but we'll take it from here.

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.

6 participants