Module talk:Sprite

Nowrap
Can the module be set up to not wrap the space between the sprite and the text? It looks terrible wherever it wraps. – KnightMiner  t/c 03:01, 17 March 2015 (UTC)


 * It does? It uses padding instead of a space so the icon should stay next to the text, while still allowing the text to wrap. –Majr ᐸ Talk Contribs 10:26, 19 March 2015 (UTC)


 * In theory it shouldn't, but it still wraps in tables, refer to Superflat (if you remove the nowrap class I added in the row "Bottomless Pit"). Also, the module does set the text part not to wrap, just not the whole thing. – KnightMiner  t/c 16:42, 19 March 2015 (UTC)


 * I guess I'll just throw a nowrap around it for now... really need to find a non-garbage way to do that, and allow text to wrap next to, or around the icon.
 * Ideally, wrapping would work something like this: Grass Block  . –Majr ᐸ Talk Contribs 13:47, 24 March 2015 (UTC)


 * Can the nowrap class be moved inside the square brackets for internal links? Otherwise it breaks link trailing, which is used in a few cases to skip setting of a title (such as on gameplay for Renewable resource). – KnightMiner  t/c 15:35, 12 May 2015 (UTC)


 * I don't see why not. –Majr ᐸ Talk Contribs 15:45, 12 May 2015 (UTC)


 * Once last thing I just thought of, if you set  in the   class, it allows wrapping between words without wrapping on the sprite. It a little less garbage of a solution as wrapping still exists next on the words, just on on the sprite. (example:    Grass Block  ). – KnightMiner  t/c 15:55, 12 May 2015 (UTC)

Merge types
I think we should merge BlockSprite, ItemSprite, EntitySprite, and EnvSprite with Sprite. I also think we should merge their link variants, including BiomeLink. ~From Contrapple 00:55, 24 March 2015 (UTC)


 * May I ask why? It seems like a lot of work for little gain. Merging would require merging the sprite sheets (which would lead to file caching issues, as well as renumbering the hundreds of IDs), images with variants in each type (for example, cauldrons) would produce the same thing, and all previous usages would have to be updated due to the ID changes.
 * Also, this talk page is generally for discussing the Module:Sprite rather than its usage, merge requests usually belong on one of the talk pages the merge is being proposed to, with optional links from the other talk pages. – KnightMiner  t/c 01:25, 24 March 2015 (UTC)
 * I put it here because doing so would mean editing this page. As for why they should be merged, I do not think there needs to be a distinction between the different types, they are all pictures of the block, item, etc. The only difference is the category, which isn't necessary and would probably use extra memory (I don't know how Lua works but I would think that removing if statements would use less memory). ~From Contrapple Grid Empty Map.png 01:40, 24 March 2015 (UTC)


 * It actually uses less memory, though I doubt it is notable, due to the other features to make it more efficient (specifically, the module is only told to load the IDs and images it actually is using, and the IDs are only loaded once per page, thus if a page only uses one sprite type, it only needs to load the sprite sheet and IDs for one type). The reason for the distinction mainly goes back to when sprite sheets were used in game, allowing them to be uploaded directly, but it also allows the convenience of editing, if the sprite sheets were combined the image would be at least three times as large.
 * Another major problem with merging all types is new sprite sheets cannot be added easily without breaking old usage due to filesize cache. Also, this module is used on templates besides those you listed, some of which you would not want within the main lists (for example, schematic and Mods/Aether/Link). So while there does not "need" to be a distinction, it makes things a lot easier. – KnightMiner  t/c 02:04, 24 March 2015 (UTC)
 * Well I agree that the templates you provided are more different, but I would not be able to understand the module without pseudo code. ~From Contrapple Grid Empty Map.png 13:16, 24 March 2015 (UTC)


 * Module:Sprite already handles all of the heavy lifting for the Sprite templates and modules; the other modules are little more than wrappers that set their own data (for example, EnvSprite contains the code, which runs Module:Sprite and tells it to load settings from Module:EnvSprite; Module:EnvSprite itself only contains an array with some general settings about the EnvSprite sprite sheet and loads Module:EnvSprite/IDs, which contains another array with the IDs associated with names for the sprites in the sheet). These could all be merged together, but all merging would accomplish is moving all of this information onto a single page, in a massive array that would be much more difficult to maintain than the current separate arrays. Any performance gains would also be negligible and would be related almost entirely to the fact that separate pages would no longer have to be loaded for a given sprite sheet.
 * If you're really looking to improve performance, I would look instead at all the string concatenations (, which, when run, would give ), since concatenation in Lua is somewhat slow and Lua strings are immutable (which means every time you change a string and store the changed value back to the same variable, Lua actually creates a copy of that variable, and all the old copies hang around until the next time it runs the garbage collector). 「 ディノ 奴  千？！ 」? · ☎ Dinoguy1000 18:30, 24 March 2015 (UTC)

Subsiution bug
I notice that when you substitute sprite or a similar template, you will display. However someone might want to be able to substitute it so they can use it in signatures.71.212.10.80 16:19, 26 April 2015 (UTC)
 * The substitution result may be too big to be allowed in signatures anyway. —  Grid Command Block.png NickTheRed37 (talk&#124;contributions) 17:52, 26 April 2015 (UTC)


 * The code is not very long, and adding support for substitution is a easy thing to do (by simply adding to the templates that call it). The problem is instead that template requires Widget:FileUrl to function, as otherwise MediaWiki will not allow files to display. While that issue is fixed by CSS on the older sheets, there still is the additional issue that we would now be using the sprite sheet without knowing where it is used, as CSS does not add file links. This would mean should the sheet change (as it is going to), there would be no way to update the old usages. I suggest instead using individual images from grid if you need a specific flair of that type. – KnightMiner  t/c 20:37, 26 April 2015 (UTC)

Non-square shapes
I think there should be a way to display rectangles as opposed to squares. The way to do this without messing up current usages would be to add a paramater for the hight (in pixels). This value would default to .71.212.10.80 20:37, 19 May 2015 (UTC)


 * There's no technical reason it can't support rectangles, but I don't see the point in adding a feature we don't have a use for. –Majr ᐸ Talk Contribs 01:25, 21 May 2015 (UTC)
 * Well we would only do it if necessary ( maybe in th future) 71.212.10.80 22:40, 1 June 2015 (UTC)

Deprecated sprites
We have gotten many sprites that have never been used or are on the wrong sprite sheet, especially on BlockSprite, and many redundant aliases which would be nice to remove, and it would be nice to be able to create a list containing those. One idea would be changing if not pos and not mw.title.getCurrentTitle.isSubpage then category = '' end to  if not pos and not mw.title.getCurrentTitle.isSubpage then category = '' elseif type( pos ) == 'table' then pos = pos[1] category = '' end which allow detecting individual names marked as deprecated (marking would be done simply by surrounding the id with curly brackets, as that was a bit simpler then actual containing a keyword in the table). Since numerical ids are generally avoided, that would match nearly all cases of deprecated sprites ID, and will match deprecated aliases as well.

On a related note, it might be relevant to add an optional category (enabled using the settings pages) to mark pages using numerical ids when string ids are available. – KnightMiner  t/c 17:34, 25 May 2015 (UTC)

Edit: Line 162 should also be changed from  to. – KnightMiner  t/c 19:42, 27 May 2015 (UTC)
 * In addition to your curly-brackets idea – which sounds good – would you be able to do a trick like this, for instance:

 ['gear'] = {71}, ['71'] = {71},
 * to be able to put the category on numbered ids?
 * – Sealbudsman (Aaron) SealbudsmanFace.png t/c 18:57, 27 May 2015 (UTC)


 * Unfortunately, that would not work with the way the module is set up, as any numerical values skip processing through the names table (numbers are simply sent to the position math as is instead). Instead, the only way to have a sprite number be deprecated would be using a separate "deprecated IDs" table, so this plan would really only work assuming pages using the template don't use numerical IDs (which we have been avoiding as that is the purpose of the names, hence my comment about a category for pages using numerical IDs) – KnightMiner  t/c 19:34, 27 May 2015 (UTC)


 * Hm. What if in, instead of having:

 if tonumber( args[1] ) then args.pos = args[1] else local default = {} ...   local pos = ids[id] or ids[mw.ustring.lower( id ):gsub( '[%s%+]', '-' )] if not pos and not mw.title.getCurrentTitle.isSubpage then category = '' end args.pos = pos end
 * we were to rearrange it like so:

 local default = {} ... local pos = ids[id] or ids[mw.ustring.lower( id ):gsub( '[%s%+]', '-' )] args.pos = pos if not pos and not mw.title.getCurrentTitle.isSubpage then if tonumber( args[1] ) then args.pos = args[1] category = '' else category = '' end elseif type( pos ) == 'table' then args.pos = pos[1] category = '' end
 * It would make it look through the table first. This would be an acceptable a performance hit, right?  Especially considering we're discouraging the use of numeric IDs?
 * – Sealbudsman (Aaron) SealbudsmanFace.png t/c 20:15, 27 May 2015 (UTC)


 * Roughly that would work for the deprecation, though note that  is to prevent from adding the category on translation pages, so it would instead need to be moved to a separate "if" function to allow the numerical positions on subpages. Also, since we generally don't use numeric IDs (except in places where those are the only IDs, which skips this code entirely), I doubt it would affect performance too much.
 * One thing to note though is that if we are advising against numerical IDs with a maintenance category, then it would be redundant to mark them for deprecation as well, as both cases would require them being fixed. That being said, it would really only make sense to either allow deprecation of numerical IDs, or to advise against all numerical IDs (basically meaning if your code is used, we would leave out the category for numerical IDs). – KnightMiner  t/c 22:19, 27 May 2015 (UTC)


 * like this?

 local default = {} ... local pos = ids[id] or ids[mw.ustring.lower( id ):gsub( '[%s%+]', '-' )] args.pos = pos if not pos then if tonumber( args[1] ) then args.pos = args[1] category = '' else category = '' end elseif type( pos ) == 'table' then args.pos = pos[1] category = '' end if mw.title.getCurrentTitle.isSubpage then category = '' end
 * If we leave in both 'deprecated sprites' and 'numeric IDs' categories, we can do the immediate job of deprecating certain sprites like colored wood, etc, and leave the idea of deprecating all numeric-ID sprites until later. Though I suppose it would be easy enough to leave out the 'numeric IDs' category line (line 8) until later as well. – Sealbudsman (Aaron) SealbudsmanFace.png t/c 22:18, 1 June 2015 (UTC)


 * That code looks just about good to me. It might be slightly faster to move "args.pos = pos" to an "else" statement at the end of the first of the "if" statement, though I don't think it is too much of a efficiency drop otherwise. (since I cannot make changes to the module, it might be nice to ping so he can state his opinion/add the changes).
 * As for the numerical thing, I agree if we are not planning on deprecating them at this time that it would be best to leave out the category for now, adding it would simply be a one line change later if requested. – KnightMiner  t/c 01:26, 2 June 2015 (UTC)