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

Add Is2k3 to Field class and fields.csv #240

Merged
merged 2 commits into from Oct 11, 2018

Conversation

Projects
None yet
4 participants
@fmatthew5876
Contributor

fmatthew5876 commented Sep 28, 2018

This adds a special is2k3 flag to fields. The flag indicates that this field is not supposed to be present in a 2k database.

This will be used in further PR's to write empty values correctly to ldb files and not write 2k3 fields to a 2k database.

There is already a bug where Terrain::footsteps is written to 2k databases. That is fixed in this changeset.

The flag could also be used in detection heuristics. For example a simple count of how many is2k3 fields exist in a given ldb file.

@fmatthew5876

This comment has been minimized.

Contributor

fmatthew5876 commented Sep 29, 2018

Added commit to not write fields to a 2k db if they are 2k3.

@Ghabry Ghabry added this to the 0.5.4 milestone Oct 2, 2018

@Ghabry

Ghabry approved these changes Oct 2, 2018

(merge after #237)

@carstene1ns

Why the change in casing of booleans TrueTRUE?

Item,spi_points2,f,Int32,0x2D,0,Integer
Item,agi_points2,f,Int32,0x2E,0,Integer
Item,using_message,f,Int32,0x33,0,Integer
Item,skill_id,f,Ref<Skill>,0x35,1,Integer - RPG2003

This comment has been minimized.

@carstene1ns

carstene1ns Oct 4, 2018

Member

So this one was wrongly identified?

This comment has been minimized.

@fmatthew5876

fmatthew5876 Oct 4, 2018

Contributor

Yes, items in rm2k can trigger skills.

SavePicture,current_sat,f,Double,0x0E,-1.0,double
SavePicture,effect_mode,f,Int32,0x0F,0,int
SavePicture,current_effect,f,Double,0x10,0.0,double
SavePicture,current_bot_trans,f,Double,0x12,0.0,double

This comment has been minimized.

@carstene1ns

carstene1ns Oct 4, 2018

Member

Seems these have been doubles before at least.

This comment has been minimized.

@fmatthew5876

fmatthew5876 Oct 4, 2018

Contributor

Excel clobbered the doubles, I was using it to make these changes.

There is also a code change in generate.py here to always write doubles as a double constant in the C++ code regardless of whether the csv has a decimal point or not.

This comment has been minimized.

@carstene1ns

carstene1ns Oct 4, 2018

Member

Yep, saw that change of course, that's why I made the comment... ;-)

@fmatthew5876

This comment has been minimized.

Contributor

fmatthew5876 commented Oct 4, 2018

Why the change in casing of booleans True → TRUE

Excel messing things up again. I can fix this back to True if you like.

@fdelapena

This comment has been minimized.

Contributor

fdelapena commented Oct 4, 2018

Excel messing things up again.

I'd prefer to keep it with existing Excel changes, then any later update with Excel will keep the diff simpler to check. Because the new column changes the whole file it is a good moment to leave these existing text case committed changes as is.

@carstene1ns

This comment has been minimized.

Member

carstene1ns commented Oct 4, 2018

Yes, have to agree with @fdelapena here. Excel is unfortunately still the defacto standard tool for such things and our generator simply ignores it.
Though generally I prefer to make the tools adapt to the data and not the data to the tools (e.g. editing code and forcing a different tab size or converting tabs to spaces - eww).

fmatthew5876 added some commits Sep 28, 2018

Add Is2k3 to fields.csv and Field class
- Also fix generate.py to always output doubles as double
@carstene1ns

This comment has been minimized.

Member

carstene1ns commented Oct 11, 2018

Since the file was converted to windows line breaks (btw. #237 changed flags.csv too :/) and so had to be "fixed" anyway, I wrote a simple linter in ruby which fixed this and the other style nits. I think about creating an issue about how certain parts of these csv's should be handled. Also we will add some .gitattributes to tell git to do line break handling here.

@carstene1ns carstene1ns merged commit 8d64bb2 into EasyRPG:master Oct 11, 2018

5 checks passed

GCW0 Build finished.
Details
GNU/Linux Build finished.
Details
OSX Build finished.
Details
Windows Build finished.
Details
web Build finished.
Details

@fmatthew5876 fmatthew5876 deleted the fmatthew5876:ldb_is2k3 branch Oct 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment