Open the-m4a opened 4 years ago
I do not own that part. so far I see people suggesting to use Workshop Uploader in Civilization 6 SDK (which can be installed from Steam). this should work for a new mod. need to check what the deal is with permissions and such. hey @Azurency , can you please share some notes on how you published this mod in the past? I want to keep this info in a file in the repo. maybe CONTRIBUTING.md
or something similar.
Anyone else see Spanish (I think?) in the City-State text?
I would submit a separate ticket (Github issue) for this. do you see this for every city state? or just this one? what is the name of the city?
I need to investigate more but no not every one, appears to be only the new ones?
So, guys, have you thought about what to do with the mod? Like what direction to go, what rules to follow, etc. A bit ot strategic thinking. I am curious because I'd like to contribute but my involvement depends on the above.
My thinking is to, at the very least, try to clean it up so that it is easier to update in the future whenever Firaxis sends out an update. I am thinking that adopting the policy of trying to reduce over-writing the Firaxis-authored versions of the Lua files will help this. And at least for the time being, it's kind of interesting work, especially to try and re-think some of the implementation strategy to avoid having code that does more work than necessary (your suggested change for the housing calculation is a good example of this).
Once we're at that point, I am not certain, but a few things come to mind:
Here are a few of my thoughts.
And last, but not least. Stop changing every tab to 2 spaces. Firaxis uses 4 spaces, and this rule is absurdly stupid.
LOL b/c while not a fan of the 2 spaces, it appeared to fit most of what had already existed with the CQUI scripts. If folks want to go to 4 spaces, I would be in favor of it as everything else I work with is 4 spaces.
I prefer the spaces over tabs b/c I work with whitespace characters enabled, which helps me with line readability, keeping things aligned and whatnot.
I also agree with some of your other points, especially in light of only a few of us doing any sort of work. Completely agree with your 2nd point - copying the entirety of the game file for a small change is frustrating whenever there's updates as it's not always easy to determine what Firaxis changed and what was changed by CQUI. Still working to get away from the full file replacements.
sorry - to be clear: I agree with all of your other points. I don't want to expand the mod, and removing the integrations of existing mods (like, if the Diplomacy Deal one is still maintained) and pointing folks at those mods would be nice
My primary use for CQUI is the build queue stuffs, and how the city screen looks. when trying to fix it back in April, that was the thing I primarily wanted to work.
Great. It seems that our thinking is aligned so I am willing to contribute. I'd like you to make me a collaborator - it will make things easier and faster.
Just realized there was a conversation going on here. Wanted to add my two cents.
Also @Infixo we have some rules setup for this repo regarding contributing. The process we have been following is generally as follows:
The code review process does slow things down somewhat but I think the way @alexeyOnGitHub set it makes the changes a bit more robust and solid.
@Infixo , I sent you an invite to join the dev team for this repo.
@alimulhuda So, what’s your approach to 4 vs. 2 spaces? I am not quite sure what you mean by asking if we are against it?
When I mentioned this, it was only to say that whatever code I would write, it would be formatted for 4 spaces, just as Firaxis does. I don’t expect anyone to systematically rewrite the code back from 2 to 4 spaces, that’s just plain waste of time. Also, I am not going to do that, unless e.g. copying some big parts of the original firaxis file because of the patch, etc.
As for the contribution process, I have a question. If I understand correctly - nobody can merge into the master without 2 reviews? And how many people are there now who can do that?
2 approvals are required from people with "WRITE" access to the repo to be able to merge a PR. an admin can still merge into "master", ignoring the 2-review requirement. everyone in "civfanatics-cqui" team has "WRITE" permissions on this repo. there are currently 3 people in that team, plus I sent you an invite too @Infixo
@Infixo Ahh okay I think i just misunderstood your comment. I thought you were advocating changing the current files into 4 space indentation. If I understand you correctly you are saying we follow a 4 spaces rule for any new files? What would you do if you were modifying an existing file which uses 2-space indentation with new functions or bugfixes?
The one thing I would be against is having mixed indentations in one file. So avoid having 4-spaced and 2-spaced in one file. I'm up for whatever you all decide, ideally project wide consistency, but if not possible then at least consistent on file by file basis.
@alimulhuda It really depends on the situation. My time is more valuable for me than having 2 and 4 spaces indents in a file. Also, people don't care how many spaces we use, they care if the mod is operational, bugs are fixed and ideally some cooperation with other mods is working. So, if I am going to change several lines out of 1000 then I will change those several lines and certainly not the entire file if it has been heavily modded so far. But if the rest can just be copied from the original, then probably I would just copy it.
on the spaces/tabs: I have been working on the CityBannerManager files this week, trying to get them boiled down to just extension-type files, and used 4 spaces instead of 2. The readability is much better with the 4 spaces. I'm happy to go with 4 spaces going forward.
FWIW, Firaxis uses tabs, which leaves the number of spaces up to the editor application in terms of displaying it as 2 spaces or 4 spaces. I don't know offhand if Notepad++ and VSCode use 2 spaces for tabs with Lua files by default... it could be that the Lua extension I have for VSCode does this. There's a setting in VSCode, Detect Indentation, that when I have it set, the tab character is displayed as 2 spaces.
When it's enabled, it sometimes enforces the 2 characters for tabs, and I haven't figured exactly what causes that, but either way:
When it's disabled, it honors the 4-spaces for the tab character:
First let me acknowledge my opinion has zero weight as I am not actually working on this but it reminds me of a time almost 20 years ago when I was working at a software company. There was a raging debate about tabs versus spaces among the developers. I was in the tabs camp for a few reasons (less typing and variable width so those who prefer 2, 3, or more spaces could adjust with their editor) and one of the guys in the other camp was so adamant about it he went around one morning and stole everyone's tab keys and taped them to his door. I don't remember which side one in the end but it seems like it was tabs because I am still using them religiously today.
Yeah - have had similar debates at work. There it was largely because the (PR) code reviews would show 8(!) spaces per tab when viewed in the browser, so it was a mess. I personally like the spaces entirely because of the dotted lines on screen, instead of the -> symbol, as I prefer working with white space characters visible.
The tab key inserting 4 spaces (instead of actual \t character) setting is one I always set as well, so my tab key does get plenty of use :)
Tabs vs spaces always leads to insane "debates" (usually just both camps sticking to their guns), which is why my personal policy is just to be consistent. If I were working on a new project by myself (only my tools and zero collaboration) then I would also do what wayneb64 does and use tabs and also set my viewer to show tab characters with size 4.
However, in group projects I think we need to stick to some conventions and I'm okay with having the granularity down to a file level scope, but no finer. The point of whitespace in a language like Lua is readability, and having inconsistent spacing screws with that. Most editors I've used for code (Visual Studio, Visual Studio Code, Notepad++) allow for a quick way to switch the amount/type of spacing used for indentation. As an aside, if you use a editor which honours the .editorconfig file then you don't even have to change your settings, it will auto-adjust based on the .editorconfig file.
Also, people don't care how many spaces we use, they care if the mod is operational, bugs are fixed and ideally some cooperation with other mods is working.
I agree that users of the mod don't care what we do internally. They also don't care about internal details like whether or not we copy files instead of extending them, but we still want to do it because it makes life easier for future development. The whitespace guidelines we agree to are not for users but for developers (current and future).
@the-m4a Originally TAB code was used to represent a tabulation of 8 columns.
Anyway, can we conclude it like this?
Some other topic, while talking about about standards. I assume that changing file names to lowercase has something to do with Linux. Can anyone of you confirm that this is the case and CQUI runs on Linux as of now?
@the-m4a Originally TAB code was used to represent a tabulation of 8 columns.
Anyway, can we conclude it like this?
1. Indenting of 4 columns done using either a TAB (then set your editors for 4 spaces) or plain 4 spaces, whatever suits you. Basically the same style as Firaxis uses.
I'm good with the 4 spaces rule. I would say no to TAB character in files.
2. If you have an opportunity and are willing to put some time into it, then gradually convert the files to their original formatting, but that is not a priority neither a requirement atm.
Agreed From what was talked about before I'm assuming @the-m4a is also in agreement. So unless @alexeyOnGitHub has any objections we can just modify the CONTRIBUTING.md file and start following that.
Regarding the lowercase thing, I don't know why it was done but it seems that people on Linux still have to go through some hoops to get the mod to work. There is an issue #36 where someone posted a workaround that is required for CQUI to work in Linux. The problem is neither @the-m4a or I play on Linux, so I'm not sure how to go about tackling that one.
I'm good with the 4 spaces rule. I would say no to TAB character in files.
I am using Notepad++ and I don't see an option to somehow autoconvert tabs to spaces. If you know how to do that then let me know pls. As for now for me it is a no go. The whole idea of using a tab is to save you clicks, not to add more and count spaces on top of that.
In Notepad++ go to Settings->Preferences...
This setting is where you can tell it to insert spaces instead of tabs whenever you press the tab key on the keyboard. If the "Replace by space" is turned on it will insert the correct number of spaces to align to the next tab stop location. So lets say you insert a space then you press tab then it will insert the remaining number of spaces (1 if tab size is set to 2 and 3 if tabsize is set to 4). If you just press tab on an empty line it will just insert the required number of spaces.
If going from tab character to spaces. Set the desired tab size from the preferences section earlier. Then use Edit->Blank Operations->TAB to Space
If going from spaces to tab characters. First set the tab size to the size used in the file (For the CQUI files right now that would be 2). Then use Edit->Blank Operations->Space to TAB (Leading). Then set the tab size to what you want in the preferences.
@Infixo Let me know if you were asking about something else in your last comment. I'm on version 7.8.6 of Notepad++. If you are on a different version and the options aren't in the same place let me know.
I man in agreement. Wouldn’t mind converting things to 4 space indent and whatnot. I will use that kind of spacing going forward
@alimulhuda Thank you for those instructions, I appreciate.
Guys, I have read your comments for the fixes I did yesterday and I am afraid that our goals and priorities are not aligned in the end. I really don't wanna spend my time explaining why I from time to time use a single line vs. multiple (there is no such rule in Lua), or maybe why I put a semicolon even when it is not needed (I do because I grew up on C/C++), or why I don't use brackets between if-then (not needed), or if there should be an empty line after end or not. And there is still a matter of the Hungarian notation, etc. And we didn't even get to really important things like code structure, reusability, objects, and new functionalities. All these just from several lines of code. It is not fun.
Here is my style: https://github.com/Infixo/Civ6-Mods/blob/master/RET/RealEraTracker.lua https://github.com/Infixo/Civ6-Mods/blob/master/REU/Lua/RealEurekas.lua https://github.com/Infixo/Real-Natural-Disasters/blob/master/Main/RealNaturalDisasters.lua If you find this code hard to read then ok. It is a subjective matter, related to your style, experience, expectations, etc. I am not going to argue about that.
As for now, please remove me from the team. And to be clear and avoid misunderstanding - like I said, I am willing to contribute, but I value my time. The amount of extra work (=time) that I am willing to put into something is proportional to the size and importance of the topic. Your current approach is too much focused on rules and compliance, creating thus an unproportionally huge amount of overhead which doesn't add value.
code styles are completely arbitrary and there is no inherent reason why one is better than the other. it that were not the case, these discussions would have ended years ago.
let's do this:
@Infixo I would be sad to see you leave, but I understand that it is a volunteering work and you don't have to do what you do not like, obviously. if your concern is about styling, this set of rules should address it.
Are there any pointers that I could use to quickly get accustomed to the codebase? I have a bunch of useful changes in mind but I've never written mods for Civ5/6. I do have lua experience.
Apologies if this is the wrong thread to ask in.
@alexeyOnGitHub We literally agreed on few things to be done differently, just a couple of posts before. Ad 1. We agreed that it is up to you and if you wanna reformat it back to 4-space-indent, then feel free to do it. If you follow current formatting then it is obvious that you will never again go back to 4-spaces as the entire CQUI is now 2-spaces. And 2-spaces is no go for me, pointed it out in the first post, and it is still a no go. "no go" as "I don't want to be forced to use 2-spaces in my code". Also, I can live with existing 2-spaces and a bit of mixture of both. Ad. 2. Was agreed. Ad. 3. See p. 1. Now all is 2-spaces, you wanna stuck with it or not? Most of us here seem to think that 4-spaces is a better choice in the long run, a) the code is more readable and b) that is what Firaxis uses. Ad. 4. Related to p. 1 and 3. Ad. 5. I will repeat - most of the code is written by Firaxis, and most of changes will be done by Firaxis. This should be the ideal we strive for if we care about consistency.
I wrote a lot of Lua code of my own, dealt with a lot Firaxis code and often deal with other people's mods and code. I am fine with all of that. The situations when the code is really unreadable usually does not come from 2 vs. 4 spaces dilemma but from things like naming, weird lua constructs, harcoding stuff, over-complicating things, not knowing API, etc. These are things that PR should be focused on imo.
@bool3max - Our contributing markdown file in the base of the repo should have some information. If it's lacking, we will add more. Basically, there's two repos of importance:
This Repo: Files found in the Assets\Expansion1 and Assets\Expansion2 folders:
Files found in Integrations are other mods that have been integrated in to CQUI.
The CQUI.modinfo describes the files that are imported. The SQL files describe sqlite database things.
So I think the update from Firaxis did not break anything it seems like it's working
still playing, and since I'm saying this I'm sure I'll find something soon
I gave it a quick try too and didn't notice anything that jumped out. I also updated the repo with the unmodded files and when I scanned over those files it seems the changes that are done with files common between the new update files and CQUI were for something called observer mode. Not sure what this observer mode is, but I think if we want to try and break CQUI we would probably have to trigger that mode and see what happens.
Just FYI - Mod is updated on Steam (I uploaded on 7/2 as well... today's contains all of the things checked in today and last few days)
Has this mod been tested with Frontier Pass yet?
Is it supported yet or does it only support the latest updates to base plus expansions 1 and 2?
I finally finished my last game and I am planing on going online and letting updates happen on my main PC tomorrow so I will at least have all the updates to the game. I am also considering paying for the new content but not if it has not been tested yet with CQUI.
@wayneb64 There is no such thing as "Frontier Pass" in the game files, setup, etc. There is the base game, exp1, exp2 and specific DLCs that add some stuff on top of that. CQUI works with version 1.0.2.39 of the the base game, exp1 and exp2.
In terms of the DLCs - there are tweaks to support Maya specific housing rules.
I have new frontier pass, and when they updated last month I hopped on to verify if things were good with it... so should anything spring up from changes via frontier pass or the patches that go with, we should hopefully be quick to respond
@Infixo @alimulhuda @alexeyOnGitHub - I noticed I can list contributors on the workshop page. When I went to pick someone, I think it only shows friends. I wonder if adding any of you as contributors would allow you to also do the workshop updates?
edit: and a quick round of the google shows me it's unlikely. I think I looked into this previously... I should not post/ask questions at crazy hours of the morning. :)
I looked through Google and yeah, people have been complaining for a long time that contributors cannot upload mods. I wasn’t aware. That’s so lame. But anyway maybe it is worth adding us to have e.g. better visibility in the comments. I actually don’t know what contributors can do besides just being listed as ones.
Is the 2 for water in Housing correct? I thought Maya didn't get a water bonus or is fresh water 2 beyond that?
@wayneb64 Please register your question / issue as a separate issue.
I don't know if it's an issue or not, so I just asked a question. If it is indeed a bug I will create the issue for it.
Me neither. But if we start solving issues or investigating specific topics, then this is no longer a “general discussion thread”, is it? “Issue” doesn’t mean that you have an actual bug. It is how things are organized, easier to track, don’t mix up with one another. There is even a label “question” for such cases.
I did some quick tests on my other PC and a Swedish city got 5 water on a river and 2 without the river. A Spanish city got 3 for water on the coast so fresh water adds three and the ocean only adds one. Net net, no issue.
I'm working on updating ML to try and unbreak the minimap on Linux/OSX.
I notice we have a customization in this file, but it doesn't seem to actually have any purpose. I looked over the git history and can see the change originally came from ML, so I have no idea why we marked it as a CQUI change! Is it OK for me to drop this change to the CheckTextureOffset in favor of what ML specifies in the current release?
@chaorace - I think it was marked as CQUI only because it differed from minimappanel.xml in the base files. So feel free to fix it up!
I pushed up all the changes since 8/2 to the workshop, huzzah
(Starting this just to cover this question): How do we get this published to the Workshop? @alexeyOnGitHub do you own that publishing mechanism? I recall that Azurency said someone else needed to do the publishing as he could not.
If we need to re-publish it ourselves, we should change the Mod ID GUID in the ModInfo file and list the old (current) GUID as an incompatible mod.