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

Updated to the Minetest location format #149 and update the Spanish #150

Closed
wants to merge 0 commits into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Mar 13, 2020

-Updated to the Minetest location format #149
-Update the Spanish

Copy link

@louisroyer louisroyer left a comment

Choose a reason for hiding this comment

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

I will review more in details later.

circular_saw.lua Outdated Show resolved Hide resolved
circular_saw.lua Outdated
@@ -366,15 +365,17 @@ function circular_saw.on_construct(pos)
local meta = minetest.get_meta(pos)
local fancy_inv = default.gui_bg..default.gui_bg_img..default.gui_slots
meta:set_string(
--For some reason the translation does not work with @n in this part

Choose a reason for hiding this comment

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

Have you tried

and

Input@nmaterial=Material de@nentrada

? This usually worked on other projects I updated translation system on…

Copy link
Author

Choose a reason for hiding this comment

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

That is what I have tried, but it does not work, it is strange because in infotext if it works

Choose a reason for hiding this comment

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

Oh, it seems related to this bug minetest/minetest#7450. And this is specific to labels… Maybe you should had a FIXME with a reference to this issue in your comment. Your workaround is fine.

@@ -158,7 +158,6 @@ minetest.register_lbm({
param2 = minetest.get_node(pos).param2

})
minetest.log('action', "LBM replaced " .. node.name ..
" at " .. minetest.pos_to_string(pos))
minetest.log('action', S("LBM replaced @1 at @2", node.name, minetest.pos_to_string(pos)))

Choose a reason for hiding this comment

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

translation does not work correctly on minetest.log output
consider removing localization on this string

Copy link
Author

Choose a reason for hiding this comment

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

What problem does it produce?, I intend to create a server and I would prefer that the logs be in Spanish

Choose a reason for hiding this comment

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

It would be fine, but currently they appear with @moreblocks) in front, and it seems strings are logged in english (translation does only work in client display).

}

stairsplus.register_single = function(category, alternate, info, modname, subname, recipeitem, fields)
local desc_base = descriptions[category]:format(fields.description)
local desc_base = S("@1 "..descriptions[category], fields.description)

Choose a reason for hiding this comment

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

It is better avoiding concatenation.

Copy link
Author

Choose a reason for hiding this comment

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

What would be the recommended way? I have removed it to avoid the error: ignoring escape sequence 'Tmoreblocks' in translation

Choose a reason for hiding this comment

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

In this kind of function, it is better when full translated description can be given as argument as in some languages descriptions[category] and fields.description are not always in the same order as in english (and also fields.description value can depend on value of descriptions[category] in languages like french) `.
It is ok to keep old parameters and adding an optional new one, and if if the full translated description is nil, fallback to not translated string else description is set to the new parameter. This implies refactoring a bit this function.

Copy link
Author

Choose a reason for hiding this comment

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

Is this what you mean?
S("@1 @2", fields.description, S(descriptions[category]))
I would have to change
@1 Microblock=Microbloque de @1
to
Microblock=Microbloque
@1 @2=@2 de @1

Choose a reason for hiding this comment

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

No, @1 @2 doesn't allow to have values of @1 and @2 depending on each other.
I mean, something like

 stairsplus.register_single = function(category, alternate, info, modname, subname, recipeitem, fields, fulldescription)

and

local desc_base = fields.description..descriptions[category]
--
-- if (…) then
--
def.description = fulldescription or ("%s (%d/16)"):format(desc_base, info)
--
-- else
--
def.description = fulldescription and S("@1 (@2/16)", fulldescription, compute_at_2()) or desc_base .. alternate:gsub("_", " "):gsub("(%a)(%S*)", function(a, b) return a:upper() .. b end)

and each something Microblock should be fully added in tr files (yes, it is a lot (they are all in stairsplus/registrations.lua), but have a look at translation files of minetest_game/stairs for reference). I can create the resulting template.txt if you want + french translation (but I can not have perfect translation in other languages because word order may varies and words may have different grammatical form depending on categories)
This means stairplus api must be changed, and all its calls… I can also help with this if you need.

Copy link
Author

Choose a reason for hiding this comment

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

The example above if it works, can't translate all the elements because it depends on the mods installed, on my server there would be 9112 blocks in moreblocks
Screenshot_20200313_151923

Choose a reason for hiding this comment

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

Can you give me some examples of mods defining nodes in moreblocks, please (or the name of your server if it is a public one)? In the next couple of days, I will try to implement a backward compatible solution allowing to have better translations if external mods define specific translation strings, but defaulting to same as your implementation. I hope, this would allow all your mods to work well in your language.

Copy link
Author

@ghost ghost Mar 13, 2020

Choose a reason for hiding this comment

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

mods:

  • bridger
  • scifi_nodes
  • concrete (technic)
  • building_blocks (homedecor_modpack)

The server is still private

Choose a reason for hiding this comment

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

Is 86502f2 working well with your mods? I have tried with technic and can reproduce your screenshot. Some strings like @1 Stair Outer still need to be updated in .tr files.

Copy link
Author

@ghost ghost Mar 14, 2020

Choose a reason for hiding this comment

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

It works fine, there are missing strings to translate, but I still don't understand what fulldescs does, I don't notice the difference, maybe in other languages.
What value would fulldescs have?
in technic fields.description = S("Concrete Post Platform") and fulldesc is nil

@Panquesito7
Copy link
Member

May I re-work this PR? It's something pending since about a year ago.

@Calinou
Copy link
Member

Calinou commented Jun 7, 2021

May I re-work this PR? It's something pending since about a year ago.

Sure 🙂

I wasn't around much to review this PR when it was origiginally made, apologies.

@ghost ghost closed this Nov 13, 2021
@ghost
Copy link
Author

ghost commented Nov 14, 2021

May I re-work this PR? It's something pending since about a year ago.

Sure slightly_smiling_face

I wasn't around much to review this PR when it was origiginally made, apologies.

Ready

@Calinou
Copy link
Member

Calinou commented Nov 14, 2021

Superseded by #186.

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

Successfully merging this pull request may close these issues.

3 participants