squidy5 / cargo_ships

factorio mod that adds cargo ships and other content to the game
25 stars 39 forks source link

Script update #66

Closed robot256 closed 2 years ago

robot256 commented 2 years ago

I saw that snouz is updating the graphics, and wanted to contribute some script bug fixes for the next release too. There are still some more bugs I want to track down, but here is what I found and fixed so far:

Todo:

snouz commented 2 years ago

Hey hi this is great ! :D It's nice to not feel alone working on it, and you're doing things I'm completely incapable of doing. Tell me when you feel ready with your changes, so we synchronize a pre-release, merging our branches.

I might have a suggestion from another mod, don't know if it really helps, maybe not: putting 'if not player or not player.valid then return end' just after declaring player.

robot256 commented 2 years ago

Happy to help! I suppose I've been doing for scripts what you're doing for graphics. Plus I have an ulterior motive, to maybe eventually slip in the code required to add Ferries and Battleships. 😈

robot256 commented 2 years ago

I might have a suggestion from another mod, don't know if it really helps, maybe not: putting 'if not player or not player.valid then return end' just after declaring player.

Everything checks for player~=nil right before using it, so that actions done by robots can be handled correctly even when player==nil. If you see a particular place it might be missing, let me know. I'm frankly kind of curious to see if we ever get a report of a LuaPlayer being invalid--maybe someone being dropped from a multiplayer game?

robot256 commented 2 years ago

Now that the map generator stuff is done, the rest is just cleanup and minor things. Let me know when your graphics are worth releasing and we can cut this PR off.

snouz commented 2 years ago

If that's ok for you, I'll push what I already have, then try to merge your work (we edited some common files, but that's ok) and we can test it to see if everything's working then ! No rush at all though, I won't publish before you're satisfied with your part !

robot256 commented 2 years ago

Okay, thanks. I will try to pull your commits into my fork and make it work that way. I may have gone overboard with the formatting changes...

BTW, I see you made several thousand 1kB files. I always wondered why spritesheets were a thing, but I have a hunch it might have to do with loading time. FIlesystem operations (including unzip, like Factorio does when loading mods) tend to take longer on more small files than fewer large ones.

The last time I messed with spritesheets, it was with a Matlab or Python script to cut them up and put them back together again. I might be able to help if you want to try gluing them back together.

snouz commented 2 years ago

No problem : ) I was currently resolving the files

I didn't know about performance, but I wrote a python to glue them back, so I could put them back

A lot of files are just in the "blender" folder, which is just a working folder.

robot256 commented 2 years ago

No problem : ) I was currently resolving the files

Style question: I reformatted a bunch of stuff with tabs just because that was what was there, and the random blocks of spaces was annoying me. But I prefer the Raigard/FLIB style guide using two spaces for indents. I'm assuming rudegrass doesn't mind how we reformat everything, so what do you think we should we standardize on? That seems to be the bulk of the conflicts.

Edit: I see that's basically what you did to control.lua. I'll make the change to my branch to hopefully make the merging easier.

Also if you want to chat about this on a Discord server we could get it mostly resolved today.

A lot of files are just in the "blender" folder, which is just a working folder.

I see 512 items in graphics/boat, 256 sprites and 256 shadows, presumably for the unrailed version. Also 256 sprites in the graphics/tanker folder. The graphics/boat/railed spritesheets look great.