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

Convert tiles and other high use lightweight classes to inline #104

Closed
GregHib opened this issue Jan 5, 2021 · 3 comments · Fixed by #125
Closed

Convert tiles and other high use lightweight classes to inline #104

GregHib opened this issue Jan 5, 2021 · 3 comments · Fixed by #125
Labels
enhancement New feature or request

Comments

@GregHib
Copy link
Owner

GregHib commented Jan 5, 2021

Blocked by https://youtrack.jetbrains.com/issue/KT-42879 due to Coordinates2D#add(x, y) and Coordinates3D#add(x, y, plane) clashing
And requires mockk/mockk#152 (comment) workaround for mockking

Classes which should be inline:
Tile, Chunk, Region, GameObject, Instance

@GregHib GregHib added the enhancement New feature or request label Jan 5, 2021
@GregHib GregHib pinned this issue Jan 30, 2021
GregHib added a commit that referenced this issue Feb 7, 2021
@GregHib
Copy link
Owner Author

GregHib commented Feb 7, 2021

Attempted; has a lot of fall out from kotlinx.serializer RC only recently supporting inline classes. Causes other compilation issues having to use IR too; kotlin scripts loose their backing interface and have to be loaded differently, loading scripts wit class loaders breaks internal classes which have to be extracted, the 3rd party kotlinx serialization Yaml library doesn't work with files over 512 in length + many more issues and work arounds.
Await full IR and inline class support.

@GregHib GregHib unpinned this issue Feb 7, 2021
@GregHib
Copy link
Owner Author

GregHib commented Feb 8, 2021

There are some work-arounds to get inline classes serializing correctly with jackson
FasterXML/jackson-module-kotlin#199

class TestSerializer : StdSerializer<Test>(Test::class.java) {
    override fun serialize(value: Test, gen: JsonGenerator, provider: SerializerProvider) {
        gen.writeNumber(value.value)
    }
}

@JsonSerialize(using = TestSerializer::class)
inline class Test(val value: Int)

data class DataThingBuilder(
    var id: Int = 0,
    var tile: Int = 0,
    var string: String = "",
    var map: Map<String, Any> = emptyMap()
) {
    fun build() = DataThing(id, Test(tile), string, map)
}

@JsonDeserialize(builder = DataThingBuilder::class)
data class DataThing(
    val id: Int = 0,
    @get:JsonProperty("tile")
    val tile: Test = Test(value = 0),
    val string: String = "",
    val map: Map<String, Any> = emptyMap()
)

@JvmStatic
fun main(args: Array<String>) {
    val mapper = ObjectMapper(YAMLFactory()).registerKotlinModule()
    val dataThing = DataThing(1234, Test(3087), "word", mapOf("anInt" to 101, "aStr" to "wow", "tile" to Test(2)))
    val string = mapper.writeValueAsString(dataThing)
    println(string)
    val data: DataThing = mapper.readValue(string)
    println(data)
    println(data.map["tile"])
    println(Test(data.map["tile"] as Int))
}

@GregHib
Copy link
Owner Author

GregHib commented Feb 8, 2021

Converted, one thing I overlooked was that bit packing values together means they can't be negative (without changing ids) meaning I had to break out a separate Delta class for negative numbers. This could be made inline too, or keep it both in Tile and keep the tile id value separate from the bit packed value.

Another side affect is GameObject's owner had to be changed from player name String to player index Int which is fine, however GameObject also needed interactionStrategy moving, which isn't optimal, it also doesn't include the despawn timer which is currently located in Objects but should be in the object itself, just like with FloorItems.

So I undid GameObjects. Yes while it would work, it's leaning too far towards entity-component-systems again.

@GregHib GregHib linked a pull request Feb 9, 2021 that will close this issue
GregHib added a commit that referenced this issue Feb 9, 2021
* Replace kotlinx.serialization with jackson

* Inline tile

* Chunk

* Region

* Instance

* Remove player init

* Delta

* Start to fix inline class tests

* Region plane

* Fix inline tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant